Flip vertical-rl coordinates for Legacy compat

Flip vertical-rl coordinates for Legacy compat

Bug: vertical-rl block layout looked identical to vertical-lr

Cause: LegacyLayout expects vertical-rl coordinates to be flipped @#$@$@!

Fix: Flip coordinates of vertical-rl blocks before setting X/Y

Personally, I'd love to eliminate legacy flipping, but it sounds like it'll be a boatload of work.
 
For context, here is what I've learned from kojii:
>>>>>
In legacy, we layout as if it's vertical-lr, then flip X. Ojan wanted to eliminate this flipping, because it's a source of bugs for paint and make code harder to read, since we need to apply different logic depending on whether the code runs before the flip and after the flip. Walter tried to do this before but it was too much work.

From you question, I guess we haven't come up with the strategy how to handle this?

I can only think two options:
Flip as the legacy does.
Allow block progression to grow in negative direction. We still don't know the right position before layout, so we need to shift after the layout.
#2 is cleaner and can eliminate flipping, but we need to allow block progression to grow in negative direction. I haven't really given enough thoughts which is better.
<<<<<
BUG=635619
Review-Url: https://codereview.chromium.org/2709103010
Cr-Commit-Position: refs/heads/master@{#453009}
Committed: https://chromium.googlesource.com/chromium/src/+/1e2c173006a9d81fd8e12b325354e7d002ff3d5c

[atotic, 2017-02-24 21:21:53]

Description was changed from

==========
Flip vertical-rl coordinates for Legacy compat

BUG=635619
==========

to

==========
Flip vertical-rl coordinates for Legacy compat

Bug: vertical-rl block layout looked identical to vertical-lr

Cause: LegacyLayout expects vertical-rl coordinates to be flipped @#$@$@!

Fix: Flip coordinates of vertical-rl blocks before setting X/Y

Personally, I'd love to eliminate legacy flipping, but it sounds like it'll be a
boatload of work.
 
For context, here is what I've learned from kojii:
>>>>>
In legacy, we layout as if it's vertical-lr, then flip X. Ojan wanted to
eliminate this flipping, because it's a source of bugs for paint and make code
harder to read, since we need to apply different logic depending on whether the
code runs before the flip and after the flip. Walter tried to do this before but
it was too much work.

From you question, I guess we haven't come up with the strategy how to handle
this?

I can only think two options:
Flip as the legacy does.
Allow block progression to grow in negative direction. We still don't know the
right position before layout, so we need to shift after the layout.
#2 is cleaner and can eliminate flipping, but we need to allow block progression
to grow in negative direction. I haven't really given enough thoughts which is
better.
<<<<<
BUG=635619
==========

[atotic, 2017-02-24 21:21:54]

atotic@chromium.org changed reviewers:
+ cbiesinger@chromium.org, eae@chromium.org, kojii@chromium.org

[atotic, 2017-02-24 21:22:07]

ptal

[cbiesinger, 2017-02-24 21:48:10]

lgtm

ah, flipped blocks :(

https://codereview.chromium.org/2709103010/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_node.cc (right):

https://codereview.chromium.org/2709103010/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_node.cc:39: LayoutUnit
container_width = containing_block->logicalHeight();
I'd do size().width() instead of logicalHeight() since we use physical
coordinates in the rest of this function

[atotic, 2017-02-24 22:40:59]

The CQ bit was checked by atotic@chromium.org

[atotic, 2017-02-24 22:40:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbiesinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2709103010/#ps20001
(title: "CR fix")

[atotic, 2017-02-24 22:41:15]

https://codereview.chromium.org/2709103010/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/ng_block_node.cc (right):

https://codereview.chromium.org/2709103010/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/ng_block_node.cc:39: LayoutUnit
container_width = containing_block->logicalHeight();
On 2017/02/24 at 21:48:10, cbiesinger wrote:
> I'd do size().width() instead of logicalHeight() since we use physical
coordinates in the rest of this function

done

[commit-bot: I haz the power, 2017-02-24 22:41:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-25 00:12:51]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487976059004450,
"parent_rev": "cfe91abf322897072fec395dd2fac85c1c89da39", "commit_rev":
"1e2c173006a9d81fd8e12b325354e7d002ff3d5c"}

[commit-bot: I haz the power, 2017-02-25 00:13:55]

Description was changed from

==========
Flip vertical-rl coordinates for Legacy compat

Bug: vertical-rl block layout looked identical to vertical-lr

Cause: LegacyLayout expects vertical-rl coordinates to be flipped @#$@$@!

Fix: Flip coordinates of vertical-rl blocks before setting X/Y

Personally, I'd love to eliminate legacy flipping, but it sounds like it'll be a
boatload of work.
 
For context, here is what I've learned from kojii:
>>>>>
In legacy, we layout as if it's vertical-lr, then flip X. Ojan wanted to
eliminate this flipping, because it's a source of bugs for paint and make code
harder to read, since we need to apply different logic depending on whether the
code runs before the flip and after the flip. Walter tried to do this before but
it was too much work.

From you question, I guess we haven't come up with the strategy how to handle
this?

I can only think two options:
Flip as the legacy does.
Allow block progression to grow in negative direction. We still don't know the
right position before layout, so we need to shift after the layout.
#2 is cleaner and can eliminate flipping, but we need to allow block progression
to grow in negative direction. I haven't really given enough thoughts which is
better.
<<<<<
BUG=635619
==========

to

==========
Flip vertical-rl coordinates for Legacy compat

Bug: vertical-rl block layout looked identical to vertical-lr

Cause: LegacyLayout expects vertical-rl coordinates to be flipped @#$@$@!

Fix: Flip coordinates of vertical-rl blocks before setting X/Y

Personally, I'd love to eliminate legacy flipping, but it sounds like it'll be a
boatload of work.
 
For context, here is what I've learned from kojii:
>>>>>
In legacy, we layout as if it's vertical-lr, then flip X. Ojan wanted to
eliminate this flipping, because it's a source of bugs for paint and make code
harder to read, since we need to apply different logic depending on whether the
code runs before the flip and after the flip. Walter tried to do this before but
it was too much work.

From you question, I guess we haven't come up with the strategy how to handle
this?

I can only think two options:
Flip as the legacy does.
Allow block progression to grow in negative direction. We still don't know the
right position before layout, so we need to shift after the layout.
#2 is cleaner and can eliminate flipping, but we need to allow block progression
to grow in negative direction. I haven't really given enough thoughts which is
better.
<<<<<
BUG=635619

Review-Url: https://codereview.chromium.org/2709103010
Cr-Commit-Position: refs/heads/master@{#453009}
Committed:
https://chromium.googlesource.com/chromium/src/+/1e2c173006a9d81fd8e12b325354...
==========

[commit-bot: I haz the power, 2017-02-25 00:13:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1e2c173006a9d81fd8e12b325354...

[eae, 2017-02-26 00:01:26]

LGTM