Bug 1544432 Comment 23 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Alex Henrie from comment #5)
> The attached patch seems to fix the problems with the Josefin Sans font, but I don't know if it's the right solution.

I suspect this is the right solution.

I think the current mozilla-central code was written with faulty assumption that the text origin ("baseline, 0" or "0, baseline" depending on writing-mode) has coordinates that are at the correct position already in the final scale.  (However, it does not.)

In one example I was looking at, we were getting a `ScaleAround` call for the rect `x=0,y=-11,width=71,height=128`, being scaled around `aPoint==(0,-5.5)`, with a scale factor of `0.01`.  This produces a very tiny point, which is anchored around `(0,-5.5)` (since that's the point we're scaling around).  I'm guessing that this `0,-5.5` point may very well be the correct the "origin" of this text (i.e. the top of its ascent), when we're dealing with the scale that we started out in -- but after we've scaled, then the 0,-5.5 position isn't really useful anymore, and it's not useful for us to have our tiny rect positioned way off entirely in negative space there.

So: I think Alex's patch (removing this ScaleAround usage, so that we just scale the whole rect around the 0,0 origin) makes sense.  I'll trigger a try run with that patch and with my automated test "part 2"-patch, just to be sure we do indeed pass the automated test everywhere after the fix -- and if that goes well, I'll land both patches.

Thanks for the report, Martin, and thanks for the patch, Alex!
(In reply to Alex Henrie from comment #5)
> The attached patch seems to fix the problems with the Josefin Sans font, but I don't know if it's the right solution.

I suspect this is the right solution.

I think the current mozilla-central code was written with faulty assumption that the text origin ("baseline, 0" or "0, baseline" depending on writing-mode) has coordinates that are at the correct position already in the final scale.  (However, it does not.)

In one example I was looking at, we were getting a `ScaleAround` call for the rect `{x=0,y=-11,width=71,height=128}`, being scaled around `aPoint==(0,-5.5)`, with a scale factor of `0.01`.  This produces a very tiny resulting rect, which is anchored around `(0,-5.5)` (since that's the point we're scaling around), and in particular it ends up producing the rect `{x = 0,y = -5.555,width = 0.71,height = 1.28}` which lives entirely in negative territory. I'm guessing that this `0,-5.5` point may very well be the correct the "origin" of this text (i.e. the top of its ascent), when we're dealing with the scale that we started out in -- but after we've scaled, then the 0,-5.5 position isn't really useful anymore, and it's not useful for us to have our tiny rect positioned way off entirely in negative space there.

So: I think Alex's patch (removing this ScaleAround usage, so that we just scale the whole rect around the 0,0 origin) makes sense.  I'll trigger a try run with that patch and with my automated test "part 2"-patch, just to be sure we do indeed pass the automated test everywhere after the fix -- and if that goes well, I'll land both patches.

Thanks for the report, Martin, and thanks for the patch, Alex!
(In reply to Alex Henrie from comment #5)
> The attached patch seems to fix the problems with the Josefin Sans font, but I don't know if it's the right solution.

I suspect this is the right solution.

I think the current mozilla-central code was written with faulty assumption that the text origin ("baseline, 0" or "0, baseline" depending on writing-mode) has coordinates that are at the correct position already in the final scale.  (However, it does not.)

In one example I was looking at, we were getting a `ScaleAround` call for the rect `{x=0,y=-11,width=71,height=128}`, being scaled around `aPoint==(0,-5.5)`, with a scale factor of `0.01`.  This produces a very tiny resulting rect, which is anchored around `(0,-5.5)` (since that's the point we're scaling around), and in particular it ends up producing the rect `{x=0,y=-5.555,width=0.71,height=1.28}` which lives entirely in negative territory. I'm guessing that this `0,-5.5` point may very well be the correct the "origin" of this text (i.e. the top of its ascent), when we're dealing with the scale that we started out in -- but after we've scaled, then the 0,-5.5 position isn't really useful anymore, and it's not useful for us to have our tiny rect positioned way off entirely in negative space there.

So: I think Alex's patch (removing this ScaleAround usage, so that we just scale the whole rect around the 0,0 origin) makes sense.  I'll trigger a try run with that patch and with my automated test "part 2"-patch, just to be sure we do indeed pass the automated test everywhere after the fix -- and if that goes well, I'll land both patches.

Thanks for the report, Martin, and thanks for the patch, Alex!

Back to Bug 1544432 Comment 23