Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Make 3D Rendering Context follow DOM tree for absolute/fixed position." #28825

Merged
merged 1 commit into from May 5, 2021

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 4, 2021

This reverts commit ed024d718b866ebc95f9eaed49a6c6dc546113d1.

Reason for revert: The original rationale that this would not change
any behavior without TransformInterop enabled was incorrect, because
backface-visibility: hidden (which does not create a containing block
for absolute/fixed) causes NeedsTransform() to be true, which thus
causes UpdateTransform to update rendering_context_id and
should_flatten_inherited_transform.

Original change's description:

Make 3D Rendering Context follow DOM tree for absolute/fixed position.

When TransformInterop is enabled, make the notion of 3D Rendering
Context follow the DOM tree for absolute and fixed-positioned elements
like it does for everything else.

When TransformInterop is not enabled, there are no differences between
following the DOM tree versus the containing block tree, which this
DCHECK()s temporarily while storing the data in two places. This is
because the objects that changes of rendering_context_id and
should_flatten_inherited_transform are associated with are all either
containing blocks for absolute and fixed positioned elements or (for
SVG) are associated closely enough with such containing blocks.

(This is intended to change behavior only when
RuntimeEnabledFeatures::TransformInteropEnabled(), though it should
(once the temporary members are removed) unconditionally reduce the
size of structs used on the stack.)

Bug: 1189985
Change-Id: I5d3bff009e2fbac00d3d011a6a9adc77ad2f9829
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2776711
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#878158}

Bug: 1189985
Fixed: 1204790, 1204853
Change-Id: Idb2a635edd9b762454bb23c536cb37bc2ac7330f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2873105
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879166}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2873105 branch from fb07b52 to a14a38c May 4, 2021
…position."

This reverts commit ed024d718b866ebc95f9eaed49a6c6dc546113d1.

Reason for revert: The original rationale that this would not change
any behavior without TransformInterop enabled was incorrect, because
backface-visibility: hidden (which does not create a containing block
for absolute/fixed) causes NeedsTransform() to be true, which thus
causes UpdateTransform to update rendering_context_id and
should_flatten_inherited_transform.

Original change's description:
> Make 3D Rendering Context follow DOM tree for absolute/fixed position.
>
> When TransformInterop is enabled, make the notion of 3D Rendering
> Context follow the DOM tree for absolute and fixed-positioned elements
> like it does for everything else.
>
> When TransformInterop is not enabled, there are no differences between
> following the DOM tree versus the containing block tree, which this
> DCHECK()s temporarily while storing the data in two places.  This is
> because the objects that changes of rendering_context_id and
> should_flatten_inherited_transform are associated with are all either
> containing blocks for absolute and fixed positioned elements or (for
> SVG) are associated closely enough with such containing blocks.
>
> (This is intended to change behavior only when
> RuntimeEnabledFeatures::TransformInteropEnabled(), though it should
> (once the temporary members are removed) unconditionally reduce the
> size of structs used on the stack.)
>
> Bug: 1189985
> Change-Id: I5d3bff009e2fbac00d3d011a6a9adc77ad2f9829
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2776711
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#878158}

Bug: 1189985
Fixed: 1204790, 1204853
Change-Id: Idb2a635edd9b762454bb23c536cb37bc2ac7330f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2873105
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879166}
@jpchase
Copy link

@jpchase jpchase commented May 5, 2021

The failing check is "without-changes", and it looks like some infra type error. Going to force merge.

@jpchase jpchase merged commit 503ab1f into master May 5, 2021
18 of 20 checks passed
18 of 20 checks passed
@community-tc-integration
sink-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
@azure-pipelines
Azure Pipelines Build #20210505.6 succeeded
Details
@azure-pipelines
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
@community-tc-integration
download-firefox-nightly Community-TC (pull_request)
Details
@community-tc-integration
lint Community-TC (pull_request)
Details
@staging-wpt-fyi
staging.wpt.fyi - firefox[experimental] Firefox results
Details
@staging-wpt-fyi
staging.wpt.fyi - safari[experimental] Safari results
Details
@community-tc-integration
wpt-chrome-dev-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-stability Community-TC (pull_request)
Details
@community-tc-integration
wpt-decision-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
@wpt-fyi
wpt.fyi - firefox[experimental] Firefox results
Details
@wpt-fyi
wpt.fyi - safari[experimental] Safari results
Details
@jpchase jpchase deleted the chromium-export-cl-2873105 branch May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants