Closed Bug 1114948 Opened 5 years ago Closed 5 years ago

Wrong positioning near the left tile boundary

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox36 --- wontfix
firefox37 --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed

People

(Reporter: nical, Assigned: nical)

Details

Attachments

(4 files)

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).
Attached video 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.
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.
Summary: When using cairo and DrawTargetTiled, glyphs that touch the left tile boundary are positioned differently → Wrong positioning near the left tile boundary
This fixes the issue on Linux.
Assignee: nobody → nical.bugzilla
Attachment #8544547 - Flags: review?(jmuizelaar)
Attachment #8544547 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/c1917197e60f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
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.
Attachment #8544547 - Flags: approval-mozilla-b2g34?
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.
Look at "Browsing Privacy", how the "v" is partly cut.
Same for "App Permissions", with the "o".
(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.
Attachment #8544547 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
You need to log in before you can comment on or make changes to this bug.