Fix blurry text #1494

ffreyer · 2021-12-02T20:49:34Z

The problem here was that the glyph padding did not make it into the texture atlas.

Makie uses a signed distance field approach, which essentially encodes the distance to the closest edge at each pixel. From that you can recover the original symbol by evaluating color = text_color * (distance > 0) at each pixel (assuming positive distances inside the symbol). Antialiasing essentially swaps the step function (distance > 0) for a smoothed step function (i.e. sigmoid). This means some small negative distances outside the original shape are included. We need to make sure that these are available, otherwise we get artifacts like in #1470.
On current master this is not the case. Padding is included in the texture generation but is discarded when the signed distance fields gets inserted into the texture atlas. This pr changes that - with it the padding is included during insertion and it's increased so that we have 1.5px padding at a textsize of 8px. (1px padding still looked blurry.)

Note that this chunks up the glyph size quite a bit. If we run into "texture atlas is too small. Resizing not implemented yet. Please file an issue at Makie if you encounter this" errors we should be fine decreasing the default glyph resolution to 32px. (In case you find this pr because of that error please do report it. You can clear your cache by deleting the files at Makie.get_cache_path() and restarting Julia or empty!(Makie.global_texture_atlas).)

Before and after:
(Textsizes are 8, 10, 16, 20, 32, 64, 100)

And specifically the figure from #1470

TODO:

include padding in texture atlas
include padding in text quad generation (screen and data space)
include padding in scatter

jkrumbiegel · 2021-12-02T21:04:16Z

I'm quite happy the bad font quality turned out to be a bug after all, thanks for having a look at this

ffreyer · 2021-12-02T21:06:15Z

GLMakie/src/gl_backend.jl

@@ -17,7 +17,6 @@ using .GLAbstraction
 const atlas_texture_cache = Dict{Any, Tuple{Texture{Float16, 2}, Function}}()

 function get_texture!(atlas)
-    Makie.set_glyph_resolution!(Makie.High)


https://github.com/JuliaPlots/Makie.jl/blob/8bab35403db92bb921738380d4c5ecff8df6ec37/WGLMakie/src/WGLMakie.jl#L49

Is this necessary? If it is we should probably have GLMakie set the glyph resolution on init, not here.

If it is we should probably have GLMakie set the glyph resolution on init, not here.

That's already the case.

Uh, I think I added this, to make sure Makie gets the correct texture atlas - sadly this is a Makie global variable... E.g., when using GLMakie, then WGLMakie and then switching back to GLMakie you'll get pretty weird artifacts

Ok, nvm, this works on an existing texture atlas...
activate! is the right place: https://github.com/JuliaPlots/Makie.jl/blob/master/GLMakie/src/GLMakie.jl#L42

I used to get artifacts when doing empty!(Makie.global_texture_atlas) and removing the cached files. But I'm not getting them anymore. I don't think I got when switching between WGLMakie and GLMakie either.

ffreyer · 2021-12-02T21:40:16Z

The test failures are the animated surface. I think we can ignore those

SimonDanisch · 2021-12-02T21:42:44Z

Amazing!
I was pretty convinced that this is a bug, since in the early days it looked just as crisp, but never got around finding the bug!

ffreyer · 2021-12-03T08:55:36Z

GLMakie/assets/shader/distance_shape.frag

@@ -41,7 +41,7 @@ flat in vec4            f_uv_texture_bbox;
 // These versions of aastep assume that `dist` is a signed distance function
 // which has been scaled to be in units of pixels.
 float aastep(float threshold1, float dist) {
-    return min(1.0, f_viewport_from_u_scale)*smoothstep(threshold1-ANTIALIAS_RADIUS, threshold1+ANTIALIAS_RADIUS, dist);


I should probably check if that changed anything. For text it was irrelevant, but maybe it matters for scatter?

I don't see a difference in characters scatters. I guess it's fine

ffreyer · 2021-12-04T11:57:41Z

The failing test is

lines(Rect(0, 0, 1, 1), linewidth=4)
scatter!([Point2f(0.5, 0.5)], markersize=1, markerspace=SceneSpace, marker='I')
current_figure()

which yields

which matches what I'd expect the marker to look like. Here it is with the render from FreeTypeAbstraction below it:

using FreeTypeAbstraction
lines(Rect(0, 0, 1, 1), linewidth=4)

font = findfont("Dejavu Sans")
char = 'I'
img, fe = renderface(font, char, 400)
w, h = size(img)
l = 0.5 - w/800
r = 0.5 + w/800
b = 0.5 - h/800
t = 0.5 + h/800
image!(l..r, b..t, img[:, end:-1:1], colormap=(:white, :black))

scatter!(
    [Point2f(0.5, 0.5)], markersize=1, markerspace=SceneSpace, marker=char, 
    color = (:orange, 0.4)
)
current_figure()

SimonDanisch · 2021-12-04T12:23:05Z

Yeah it's not unlikely, that we should just update the reference images for that .. we should take the chance and pin down the expected behavior

ffreyer · 2021-12-04T12:41:45Z

The short version of my changes is that a real numbered markersize with a character or string marker is now drawn as scaled version of the respective character. I.e. scatter matches in size with text if the markersize matches textsize. The old version scaled the quad without caring about what the aspect ratio the glyph has, stretching things like I.

The scaling changes result in this:

scene = Scene()
campixel!(scene)
scatter!(scene, Point2f[(50, 100), (100, 100), (150, 100), (200, 100)], marker="^l_j", markersize=50)
scatter!(scene, Point2f[(50, 100), (100, 100), (150, 100), (200, 100)], color = :red, markersize=4)
text!(scene, ["^", "l", "_", "j"], position = Point2f[(50, 200), (100, 200), (150, 200), (200, 200)], textsize = 50, align=(:center, :center))
scene

Old/master version:

jkrumbiegel · 2021-12-04T12:53:22Z

Perfect, I think that matches how I implemented it in Cairomakie

ffreyer · 2021-12-04T13:19:40Z

Up to maybe a pixel or two the "^l_j" example is the same between CairoMakie, GLMakie and WGLMakie now. Though CairoMakie can't do string markers, only characters so I switched to 4 individual scatter plots.

I think from my side this is done now. (short_test_13 should fail on every run now, but have the same as the one I showed above.)

SimonDanisch · 2021-12-06T12:59:16Z

Sweet!!!!
Do you want to add the test that generated

to maybe:
https://github.com/JuliaPlots/Makie.jl/blob/master/ReferenceTests/src/tests/text.jl
?
I'll update the reference images after we merged everything!

ffreyer · 2021-12-06T13:18:42Z

I was using https://github.com/JuliaPlots/Makie.jl/blob/cb9087ee9055313260611e7eafa8e7a0a0184024/GLMakie/assets/shader/distance_shape.frag#L158-L160
for those, so getting the bounding boxes for a ref image test isn't really possible. We could do it without, but there's already https://github.com/JuliaPlots/Makie.jl/blob/cb9087ee9055313260611e7eafa8e7a0a0184024/ReferenceTests/src/tests/short_tests.jl#L13-L17
which more or less checks the same thing?

Without bounding boxes:

SimonDanisch · 2021-12-06T13:21:46Z

Ah I see... Would be nice to test those boundingboxes somewhere still, to see if they match what freetype expects...
Lets merge this regardless! Thanks for the great effort & detective work to fix this!! :)

ffreyer · 2021-12-06T13:29:42Z

They don't because they have padding :p

I guess you could run a character through the text machinery and then check if the text_quads output matches the size from FreeTypeAbstraction with pixelsize * 2 * GLYPH_PADDING[] / PIXELSIZE_IN_ATLAS[] padding?

jonas-schulze · 2021-12-06T13:31:57Z

Nice work, thank you!

SimonDanisch · 2021-12-06T13:32:28Z

Well, we do need a function that correctly gives us the rendered boundingbox of a glyph for a certain font & fontsize ... I lost a bit track which one that is right now, but maybe that's a good chance to find the correct one and document & test it!

ffreyer · 2021-12-06T13:39:14Z

Ok I'll give it a try

ffreyer added 2 commits December 2, 2021 21:19

correctly include glyph padding

530cc27

let the user set glyph resolution

1fdd3c8

ffreyer commented Dec 2, 2021

View reviewed changes

ffreyer commented Dec 3, 2021

View reviewed changes

fix glyph scale and offset in scatter

939bc78

fix glyph scaling in WGLMakie

0ce4837

ffreyer added 2 commits December 4, 2021 14:07

include short_test_13 (char scatter)

4ad5ea5

include short_test13 in CairoMakie (char marker)

9060989

SimonDanisch merged commit f233cda into master Dec 6, 2021

SimonDanisch deleted the ff/text branch December 6, 2021 13:22

bjarthur mentioned this pull request Sep 1, 2023

Using many polygon shapes for scatter runs into texture atlas limit #3204

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix blurry text #1494

Fix blurry text #1494

ffreyer commented Dec 2, 2021 •

edited

Loading

jkrumbiegel commented Dec 2, 2021

ffreyer Dec 2, 2021

ffreyer Dec 2, 2021

SimonDanisch Dec 6, 2021

SimonDanisch Dec 6, 2021

ffreyer Dec 6, 2021

ffreyer commented Dec 2, 2021

SimonDanisch commented Dec 2, 2021

ffreyer Dec 3, 2021

ffreyer Dec 4, 2021

ffreyer commented Dec 4, 2021

SimonDanisch commented Dec 4, 2021

ffreyer commented Dec 4, 2021 •

edited

Loading

jkrumbiegel commented Dec 4, 2021

ffreyer commented Dec 4, 2021 •

edited

Loading

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

jonas-schulze commented Dec 6, 2021

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

Fix blurry text #1494

Fix blurry text #1494

Conversation

ffreyer commented Dec 2, 2021 • edited Loading

jkrumbiegel commented Dec 2, 2021

ffreyer Dec 2, 2021

Choose a reason for hiding this comment

ffreyer Dec 2, 2021

Choose a reason for hiding this comment

SimonDanisch Dec 6, 2021

Choose a reason for hiding this comment

SimonDanisch Dec 6, 2021

Choose a reason for hiding this comment

ffreyer Dec 6, 2021

Choose a reason for hiding this comment

ffreyer commented Dec 2, 2021

SimonDanisch commented Dec 2, 2021

ffreyer Dec 3, 2021

Choose a reason for hiding this comment

ffreyer Dec 4, 2021

Choose a reason for hiding this comment

ffreyer commented Dec 4, 2021

SimonDanisch commented Dec 4, 2021

ffreyer commented Dec 4, 2021 • edited Loading

jkrumbiegel commented Dec 4, 2021

ffreyer commented Dec 4, 2021 • edited Loading

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

jonas-schulze commented Dec 6, 2021

SimonDanisch commented Dec 6, 2021

ffreyer commented Dec 6, 2021

ffreyer commented Dec 2, 2021 •

edited

Loading

ffreyer commented Dec 4, 2021 •

edited

Loading

ffreyer commented Dec 4, 2021 •

edited

Loading