1114948 - Wrong positioning near the left tile boundary

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Nicolas Silva [:nical]]

A more precise wording would probably be "glpyphs that touch the left DrawTarget boundary" but this is especially noticeable with tiling, since a glyph that appears on two tiles will sometimes look slightly broken (half of the glyph being offset by one pixel (or a fraction of a pixel) to the left).

This issue doesn't reproduce with the skia backend, and is the main blocker for DrawTargetTiled on android (caught by a reftest).

Comment 1

[Nicolas Silva [:nical]]

Attachment: screencast

This shows the bug with the right prefs on Linux. Notice how the letters that are close to the tile boundary are shaking when the text moves.

Comment 2

[Nicolas Silva [:nical]]

Turns out it's not specific to glyphs, nor to cairo. We use NS_roundf in the tiling code with various things like the rectangles to copy between the front and the back buffer, and the valid regions. NS_roundf doesn't behave well around 0 in our use case. We actually should be using NS_lroundf which is more consistent when passing from positive to negative.

Comment 3

[Nicolas Silva [:nical]]

Attachment: Use NS_lroundf instead of NS_roundf in the tiling code

This fixes the issue on Linux.

Comment 4

[Nicolas Silva [:nical]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1917197e60f

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c1917197e60f

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8544547 [details] [diff] [review]
Use NS_lroundf instead of NS_roundf in the tiling code

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: subtle but frustrating display issue on some pages
Testing completed: yes, manually on master
Risk to taking this patch (and alternatives if risky): low -- it's on 2.2 and master for more than 1 month.
String or UUID changes made by this patch: none

It's kinda late to request an uplift to v2.1, but I see this too often on my 2.1 dogfooding device, and it's not nice. This is really polish, nothing is broken, but this is quite visible because happens in the middle of the screen.

I'll share one screenshot with the issue.

Comment 7

[Julien Wajsberg [:julienw]]

Attachment: french language, in Settings

Look in the middle of the screen, the word 'disponibles' below 'Stockage des applications'. The "l" has one vertical line missing.

Same issue on the line above, same word, but the "b" has the issue. (it just happens it's on the same vertical line).

Looking closely, you can see the "B" letter of "USB" in "Stockage USB" also has a thin line missing in the extreme right.

Comment 8

[Julien Wajsberg [:julienw]]

Attachment: english language, settings app

Look at "Browsing Privacy", how the "v" is partly cut.
Same for "App Permissions", with the "o".

Comment 9

[Nicolas Silva [:nical]]

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Comment on attachment 8544547 [details] [diff] [review]
> Use NS_lroundf instead of NS_roundf in the tiling code
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> 
> [Approval Request Comment]

FWIW, this patch is trivial to uplift and and really not risky.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/1cf422858f5c

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1cf422858f5c