Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for rendering image markers with CairoMakie #2080

Merged
merged 19 commits into from
Aug 24, 2022

Conversation

fatteneder
Copy link
Contributor

@fatteneder fatteneder commented Jun 20, 2022

Description

Overload CairoMakie.draw_marker for Matrix{<:Colorant} types which allows to render images as markers.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@fatteneder
Copy link
Contributor Author

Tests are failing because of a failure in the conversion pipeline for the markers.

I retraced it to this PR #1981 which introduced the _marker_convert method that is now erroring:

# an array of markers is converted to string by itself, which is inconvenient for the iteration logic
_marker_convert(markers::AbstractArray) = map(m -> convert_attribute(m, key"marker"(), key"scatter"()), markers)
_marker_convert(marker) = convert_attribute(marker, key"marker"(), key"scatter"())

The issue is that when scattering with image markers you pass a marker of type Matrix{<:Colorant} <: AbstractArray for which _marker_convert tries to convert all elements as markers. I think adjusting the overload here should do the trick.

Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! Couple of minor changes requested.

CairoMakie/src/primitives.jl Outdated Show resolved Hide resolved
CairoMakie/src/primitives.jl Outdated Show resolved Hide resolved
@asinghvi17
Copy link
Member

asinghvi17 commented Jun 21, 2022

Oops...I guess this breaks WGLMakie. Maybe we could exclude this test from there?

https://github.com/JuliaPlots/Makie.jl/blob/216a353a6c6f0c39078eb21d8a80e954eae39ad5/WGLMakie/test/runtests.jl#L41

@fatteneder
Copy link
Contributor Author

Can't we fix this right now? Although I would not know how 😅

@SimonDanisch
Copy link
Member

Is this really just for PDF, or for CairoMakie? Looks to me like the latter, no?

@fatteneder
Copy link
Contributor Author

Aeeehhh, could be 😅. I only tested with PDFs. I guess @asinghvi17's 👍 to your answer indicates that it also works for CairoMakie. I'll update the news then.

Honestly, I am don't know yet which parts of Makie are responsible for saving scenes to files? And what file formats it can handle?
So far I only used CairoMakie to generate PDFs. Does it also work with SVGs and PNGs? Perhaps I should read the docs more carefully 😆

@fatteneder fatteneder changed the title Support for rendering image markers to PDF Support for rendering image markers with CairoMakie Jun 21, 2022
@asinghvi17
Copy link
Member

asinghvi17 commented Jun 21, 2022

Basically, you can think of it this way - Makie specifies the points to render, and the backend packages actually render them.

CairoMakie works with PNG and vector types (SVG, EPS, PDF). [W]GLMakie works only with bitmap/raster types (fundamentally PNG) and conversions to jpg, bmp, tiff, etc.

For example, a complex recipe will be degraded to atomic plots (lines, scatter, heatmap/image, surface, mesh, meshscatter, volume) by Makie. The backend's basic task is to render these atomic plots in the correct positions. Of course, you can then hook into higher level recipes to render those directly if you want, as CairoMakie does with poly. GLMakie renders only atomic level plots to OpenGL, WGLMakie renders them to browser. At one point we had a GRMakie which was trying to render Makie plots with GR, but it lost momentum and the packages weren't really that compatible.

@asinghvi17
Copy link
Member

GLMakie CI actually passes, but since this post is from an external PR I guess the bot can't comment that there is a missing refimage. Otherwise this is good to go.

@SimonDanisch SimonDanisch reopened this Jul 8, 2022
Copy link
Member

@asinghvi17 asinghvi17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are some bugs here - shouldn't be too difficult to fix though.

Left is reference image, right is recorded image - this is from the Image scatter different sizes example.
Screen Shot 2022-07-12 at 6 25 59 PM

The scale of the bottom marker looks weird and the top marker seems (a) misaligned and (b) somehow more compressed?

Screen Shot 2022-07-12 at 6 25 37 PM

More detail on the top marker weirdness

CairoMakie/src/primitives.jl Outdated Show resolved Hide resolved
CairoMakie/src/primitives.jl Outdated Show resolved Hide resolved
@fatteneder
Copy link
Contributor Author

I pushed a fix for the wrong scaling, at least now the Makie logo looks ok (the problem were swapped dimensions due to the permutedims call).

However, the logo and the doggy now both appear smaller than in the ref image (as you already noted above).
I don't know how to fix that given only the size of the picture and the scale factor.
Adding an artificial factor of 3/2 to the scaling (and offset) would give roughly the right sizes.
Any idea where this difference could origin from?

@asinghvi17
Copy link
Member

Any idea where this difference could origin from?

Could you try rendering with different px_per_units and see what happens? My guess is that somehow you are drawing in device space, not Cairo's user space. Especially since the reference images seem to be the same.

@fatteneder
Copy link
Contributor Author

Could you try rendering with different px_per_units and see what happens?

I already tried with different scaling. E.g. adding an artificial factor of 3/2 gives roughly the same size as the ref image.

My guess is that somehow you are drawing in device space, not Cairo's user space. Especially since the reference images seem to be the same.

Perhaps it is the difference between markerspace = :data, :pixel and the test needs to be updated?
Using markerspace=:data seems to work fine in terms of sizing and positioning:

using FileIO
using CairoMakie
CairoMakie.activate!()
using Makie

img = Makie.logo()
img2 = load(Makie.assetpath("doge.png"))
images = [img, img2]
# markersize = map(img-> Vec2f(reverse(size(img) ./ 10)), images)
markersize = [ ( 1.0,0.5 ), ( 1.0,1.0 ) ]
s = scatter(1:2, [0.75,2], marker = images, markersize=markersize, axis=(limits=(0.5
            markerspace=:data)

save("scatter.pdf", s)

image

@fatteneder
Copy link
Contributor Author

fatteneder commented Jul 13, 2022

Could you try rendering with different px_per_units and see what happens?

I already tried with different scaling. E.g. adding an artificial factor of 3/2 gives roughly the same size as the ref image.

Sorry, did not notice that there is an actual px_per_unit/pt_per_unit attribute for save.
Changing it does not change anything in the output though.

@fatteneder
Copy link
Contributor Author

I spent more time on testing and finally compared the png outputs generate by GLMakie and CairoMakie:

using FileIO
using Makie

function test_scatter(filename)
  img = Makie.logo()
  img2 = load(Makie.assetpath("doge.png"))
  images = [img, img2]
  markersize = map(img-> Vec2f(reverse(size(img) ./ 10)), images)
  s = scatter(1:2, 1:2, marker = images, markersize=markersize, axis=(limits=(0.5, 2.5, 0.5, 2.5),))
  save(joinpath(@__DIR__, filename), s)
end

using CairoMakie
CairoMakie.activate!()
test_scatter("cairo_scatter.png")

using GLMakie
GLMakie.activate!()
test_scatter("gl_scatter.png")
  • GLMakie output:
    gl_scatter

  • Cairo output:
    cairo_scatter

Booth pictures are identically, but they differ from the test output of test CairoMakie:
Image Scatter different sizes

I guess the difference is in different settings in the test, but I could not figure out what those are. Where can I find them?

But since the tests now pass I guess the sizing is not the issue anymore. Only the colored edges in pdf output remain.

@fatteneder
Copy link
Contributor Author

I opened an issue for the colored edges problem since it also appears when saving it as pdf using an image plot.

This PR should now be good to go.

@SimonDanisch
Copy link
Member

Thank you all! :)

@SimonDanisch
Copy link
Member

Just noticed, that the added test never worked in CairoMakie:
image-scatter
Was this working at some point in this PR?

@fatteneder
Copy link
Contributor Author

Might be that I forgot to check the CairoMakie result, my bad.
I will take a look.

@fatteneder fatteneder mentioned this pull request Sep 11, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants