resize reflow optimizations need to be aware of writing mode and behave accordingly

RESOLVED FIXED in mozilla37

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Blocks 2 bugs)

unspecified
mozilla37
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

5 years ago
E.g. when resizing in the vertical direction only, we currently avoid re-doing line breaking because we know the block *width* is not changing. But for vertical writing mode, this is obviously wrong; this optimization would apply to a horizontal resize instead.
Blocks: 1079125
(Assignee)

Updated

5 years ago
(Assignee)

Comment 1

5 years ago
Here's a simple testcase. Try resizing the window in the vertical direction only, and note how the text in the upper <div> does not rewrap as expected. Interestingly, in the lower <div>, where the text is in <p> elements, it does rewrap; only the case with text contained directly in the <div> fails.

When resizing the window in both dimensions at once (by its corner), all the text reflows fine. Or after doing a vertical resize (so that the upper div has stale lines), if you then resize slightly in the horizontal direction everything will snap back into place properly, as we reflow the vertical lines.
(Assignee)

Comment 2

5 years ago
I'm sure there will be other places requiring similar writing-mode checks, but this is the most blatant case, I think; fixing this makes lines reflow as expected when resizing the attached testcase. (And we don't reflow the lines when resizing only horizontally.)
Attachment #8529268 - Flags: review?(smontagu)
(Assignee)

Updated

5 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Comment on attachment 8529268 [details] [diff] [review]
part 1 - nsBlockFrame::Reflow needs to take account of writing mode when deciding if a resize reflow requires dirtying its lines

Cancelling r? here for now, as I'm working on a better patch that will incorporate this.
Attachment #8529268 - Flags: review?(smontagu)
(Assignee)

Comment 4

5 years ago
I think what we should ultimately do is to convert the mHResize/mVResize flags to logical mode, but it'll take a while to adapt all users. So as a first step, this patch provides both logical and physical accessor functions, and adapts all users of the flags to use the accessor methods instead. This will allow us to convert callers piecemeal, and eventually switch the stored form of the flags if that seems optimal.
Attachment #8529763 - Flags: review?(smontagu)
(Assignee)

Comment 5

5 years ago
Now it's trivial to convert the usage in nsBlockFrame, which fixes the resizing testcase here.
Attachment #8529764 - Flags: review?(smontagu)
Attachment #8529763 - Flags: review?(smontagu) → review+
Attachment #8529764 - Flags: review?(smontagu) → review+
Depends on: 1106083
https://hg.mozilla.org/mozilla-central/rev/733eb2aff2ce
https://hg.mozilla.org/mozilla-central/rev/2548c7a4a66a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.