bounding box returned by a text run does not account for synthetic-italic glyphs that may project beyond the original glyph bounding box

RESOLVED FIXED in mozilla37

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla37
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-chrome])

Attachments

(2 attachments, 1 obsolete attachment)

See bug 1116315 for a screenshot and simple testcase based on the Gaia SMS app.
Blocks: 1116315
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8542680 [details] [diff] [review]
gfxTextRun::Measure needs to account for fake-italic transform when returning the bounding box

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

::: gfx/thebes/gfxFont.cpp
@@ +2276,5 @@
> +    // bottom-left being skewed further left.
> +    if (mStyle.style != NS_FONT_STYLE_NORMAL && !mFontEntry->IsItalic()) {
> +        metrics.mBoundingBox.width += 0.3 * metrics.mBoundingBox.height;
> +        metrics.mBoundingBox.x -=
> +          0.3 * (metrics.mBoundingBox.height + metrics.mBoundingBox.y);

metrics.mBoundingBox.YMost()

While these seem correct, the derivation is non-trivial and I think we should make the code clearer. Also, I believe these are supposed to be nscoords so we need to do some floor/ceil. So write something like
  gfxFloat extendLeftEdge = ceil(0.3 * metrics.mBoundingBox.YMost());
  gfxFloat extendRightEdge = ceil(0.3 * -metrics.mBoundingBox.y);
  metrics.mBoundingBox.width += extendLeftEdge + extendRightEdge;
  metrics.mBoundingBox.x -= extendLeftEdge;
Attachment #8542680 - Flags: review?(roc) → review-
Updated according to comment 3. I also thought we should do something about the "magic number" used for the skew factor, which is currently defined in various places across the different backends; moved that to a single location in gfxFont.h.
Attachment #8542944 - Flags: review?(roc)
Attachment #8542680 - Attachment is obsolete: true
Comment on attachment 8542944 [details] [diff] [review]
gfxTextRun::Measure needs to account for fake-italic transform when returning the bounding box

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

Nice!
Attachment #8542944 - Flags: review?(roc) → review+
According to my finding in bug 1116315 comment 17.
Whiteboard: [parity-chrome]
You need to log in before you can comment on or make changes to this bug.