fix: Offset text regions that are out of viewport #6986
Conversation
avelad
added
type: bug
avelad
force-pushed
the
UITextDisplayer-clip-regions
branch
from
0da4587 to
0ac98d8
Compare
|
Incremental code coverage: 95.00% |
avelad
force-pushed
the
UITextDisplayer-clip-regions
branch
from
0ac98d8 to
349aac1
Compare
| @@ -504,6 +508,19 @@ shaka.text.UITextDisplayer = class { | |||
| regionElement.style.left = region.viewportAnchorX - | |||
| region.regionAnchorX * region.width / 100 + viewportAnchorUnit; | |||
There was a problem hiding this comment.
while the below is probably fine, in trying to figure out what was going on I realized that this and section uses region.height/width but doesn't account for whether it's in line units of precentages. This ends up working out for left, but for top we end up with 95% but it should be 77.68%, which is what we'd get if we multiply the height by the then lineHeightMultiple. We potentially should store the calculated height and width from above in a var so that we can use it here and below, if we wanted to keep that. Though, potentially shouldn't be necessary.
There was a problem hiding this comment.
@gkatsev Can you create a PR with it? Thanks!
| @@ -504,6 +508,19 @@ shaka.text.UITextDisplayer = class { | |||
| regionElement.style.left = region.viewportAnchorX - | |||
| region.regionAnchorX * region.width / 100 + viewportAnchorUnit; | |||
| } | |||
| if (region.heightUnits !== pixelUnit && | |||
There was a problem hiding this comment.
I don't get why you are checking, for these things, !== pixelUnit, instead of checking === percentageUnit.
That would suggest that this code works for both percentages and lines.
This code assumes that the max for width and height are both 100, which is true for percentages, but lines max out at 16 or 32 depending on the aspect ratio.
There was a problem hiding this comment.
Internally we convert the lines to % that's why I don't check percentages
avelad
changed the title
Fixes #3732 (we resolve it using offset operation instead of clipping)
Fixes #3732 (we resolve it using offset operation instead of clipping)
Fixes #3732 (we resolve it using offset operation instead of clipping)