Closed Bug 1117210 Opened 5 years ago Closed 5 years ago

snap text baseline to device pixels in vertical text (like we do for horizontal text)

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

In the initial vertical-text patches, I didn't bother with snapping baseline positions to device pixels (which we do with nsLayoutUtils::GetSnappedBaselineY for horizontal text). We should fix this, for more consistent layout.
This handles vertical baseline snapping similarly to the existing code for horizontal text.
Attachment #8543401 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Here's a reftest for baseline snapping; this fails on OS X and Windows/D2D with current code, and passes with the patch.
Attachment #8543402 - Flags: review?(smontagu)
Comment on attachment 8543402 [details] [diff] [review]
Reftest for baseline-snapping in vertical writing modes

Review of attachment 8543402 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/writing-mode/reftest.list
@@ +2,5 @@
>  # It should not be included in layout/reftests/reftest.list until vertical layout
>  # is turned on
>  == 1082844.html 1082844-ref.html
>  == 1083748.html 1083748-ref.html
> +fuzzy(53,11) HTTP(..) == 1083848-1-inline-border.html 1083848-1-inline-border-ref.html

Is this from some other patch, or does this fix make things fuzzier? That seems counter-productive prima facie.
Comment on attachment 8543401 [details] [diff] [review]
Snap text baselines to device pixels in vertical writing modes

Review of attachment 8543401 [details] [diff] [review]:
-----------------------------------------------------------------

This seems to make sense.
Attachment #8543401 - Flags: review?(smontagu) → review+
(In reply to Simon Montagu :smontagu from comment #3)
> Comment on attachment 8543402 [details] [diff] [review]
> Reftest for baseline-snapping in vertical writing modes
> 
> Review of attachment 8543402 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/reftests/writing-mode/reftest.list
> @@ +2,5 @@
> >  # It should not be included in layout/reftests/reftest.list until vertical layout
> >  # is turned on
> >  == 1082844.html 1082844-ref.html
> >  == 1083748.html 1083748-ref.html
> > +fuzzy(53,11) HTTP(..) == 1083848-1-inline-border.html 1083848-1-inline-border-ref.html
> 
> Is this from some other patch, or does this fix make things fuzzier? That
> seems counter-productive prima facie.

Oops, sorry -- that didn't belong here, and is no longer present in my local patch queue. Consider it reverted!
Here's the correct reftest patch, without extraneous fuzz. :)
Attachment #8562065 - Flags: review?(smontagu)
Attachment #8543402 - Attachment is obsolete: true
Attachment #8543402 - Flags: review?(smontagu)
Comment on attachment 8562065 [details] [diff] [review]
Reftest for baseline-snapping in vertical writing modes

Review of attachment 8562065 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8562065 - Flags: review?(smontagu) → review+
Re-landed this, as the breakage there was caused by bug 1082017 rather than this bug.

https://hg.mozilla.org/integration/mozilla-inbound/rev/53ca9d080af4
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef7af73497de
Flags: needinfo?(jfkthame)
https://hg.mozilla.org/mozilla-central/rev/53ca9d080af4
https://hg.mozilla.org/mozilla-central/rev/ef7af73497de
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.