SET: Match root with shared element in animations. [chromium/src : main]
0 views
Skip to first unread message
Vladimir Levin (Gerrit)
Jun 24, 2022, 10:56:55 PM6/24/22
to blink-revie...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, Khushal Sagar, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Attention is currently required from: Khushal Sagar.
1 comment:
Patchset:
Please take a look.
To view, visit change 3722517. To unsubscribe, or for help writing mail filters, visit settings.
Khushal Sagar (Gerrit)
Jun 27, 2022, 11:36:34 AM6/27/22
to Vladimir Levin, blink-revie...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Attention is currently required from: Vladimir Levin.
2 comments:
File third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc:
gfx::Rect cached_border_box_in_css_space = gfx::Rect(gfx::Size(
element_data->cached_border_box_size_in_css_space.Width().ToInt(),
element_data->cached_border_box_size_in_css_space.Height().ToInt()));Move this into the if block for animations since that's the only code using it.
append_selector("html::page-transition-container", document_transition_tag);
builder.AppendFormat(
R"CSS({
width: %dpx;
height: %dpx;
transform: %s;
writing-mode: %s;
})CSS",
border_box_in_css_space.width(), border_box_in_css_space.height(),
ComputedStyleUtils::ValueForTransformationMatrix(
element_data->viewport_matrix, 1, false)
->CssText()
.Utf8()
.c_str(),
writing_mode_stream.str().c_str());- Old and new root exist. We won't be in this loop. AllRootTags() will update the style for the container and we'll get an automatic cross-fade. *Add a TODO to retarget when the new viewport size changes.*
- Old root and no shared element (exit animation for root). Then we shouldn't be in this loop since it won't be in element data. The style is set up in AllRootTags() and fade-out.
- Old root and shared element. This style updates the container for the new shared element size and transform. The code below adds an animation from old root size and identity matrix. *We shouldn't visit this in AllRootTags iteration above but only after its been matched*
- New root and no shared element. Entry animation for new root. Its only visited in AllRootTags and its a default fade-in.
- New root and old shared element. We correctly visit it in AllRootTags. *We need to skip setting the styles here since latest value comes from AllRootTags*. But we do have to set the animation below.
Other than the above (please put a comment in the code), we need to set the writing-mode for containers with root tag.
To view, visit change 3722517. To unsubscribe, or for help writing mail filters, visit settings.
Vladimir Levin (Gerrit)
Jun 27, 2022, 5:29:55 PM6/27/22
to blink-revie...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, Khushal Sagar, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Attention is currently required from: Khushal Sagar.
2 comments:
File third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc:
gfx::Rect cached_border_box_in_css_space = gfx::Rect(gfx::Size(
element_data->cached_border_box_size_in_css_space.Width().ToInt(),
element_data->cached_border_box_size_in_css_space.Height().ToInt()));Move this into the if block for animations since that's the only code using it.
Done
append_selector("html::page-transition-container", document_transition_tag);
builder.AppendFormat(
R"CSS({
width: %dpx;
height: %dpx;
transform: %s;
writing-mode: %s;
})CSS",
border_box_in_css_space.width(), border_box_in_css_space.height(),
ComputedStyleUtils::ValueForTransformationMatrix(
element_data->viewport_matrix, 1, false)
->CssText()
.Utf8()
.c_str(),
writing_mode_stream.str().c_str());
- Old and new root exist. We won't be in this loop. […]
Done
To view, visit change 3722517. To unsubscribe, or for help writing mail filters, visit settings.
Khushal Sagar (Gerrit)
Jun 28, 2022, 11:17:48 AM6/28/22
to Vladimir Levin, blink-revie...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Attention is currently required from: Vladimir Levin.
Patch set 6:Code-Review +1
4 comments:
Patchset:
Please consider adding a test which validates the incoming snapshot too. :)
File third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc:
// TODO(khushalsagar) : The duration/delay in the UA stylesheet will
// need to be the duration from TransitionConfig. See crbug.com/1275727.This can go away now, we're way past that API. :)
nit: const here and below.
File third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-end-ref.html:
#box {
width: 400px;
height: 400px;
contain: paint;
}This is probably redundant?
To view, visit change 3722517. To unsubscribe, or for help writing mail filters, visit settings.
Vladimir Levin (Gerrit)
Jun 28, 2022, 1:15:31 PM6/28/22
to blink-revie...@chromium.org, blink-...@chromium.org, Khushal Sagar, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Patch set 8:Commit-Queue +2
4 comments:
Patchset:
Added another test.
File third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc:
// TODO(khushalsagar) : The duration/delay in the UA stylesheet will
// need to be the duration from TransitionConfig. See crbug.com/1275727.
This can go away now, we're way past that API. […]
Done
nit: const here and below.
Done
File third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-end-ref.html:
#box {
width: 400px;
height: 400px;
contain: paint;
}This is probably redundant?
Done
To view, visit change 3722517. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Jun 28, 2022, 7:10:11 PM6/28/22
to Vladimir Levin, blink-revie...@chromium.org, blink-...@chromium.org, Khushal Sagar, Alexis Menard, chromium...@chromium.org, Olga Gerchikov
Chromium LUCI CQ submitted this change.
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-incoming-ref.html
Insertions: 13, Deletions: 0.
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<title>Root to shared animation test (ref)</title>
+<link rel="help" href="https://github.com/WICG/shared-element-transitions">
+<link rel="author" href="mailto:vmp...@chromium.org">
+
+<style>
+body {
+ background: blue;
+ padding: 0;
+ margin: 0;
+}
+
```
```
The name of the file: third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-end-ref.html
Insertions: 2, Deletions: 9.
@@ -9,11 +9,6 @@
body {
background: yellow;
}
-#box {
- width: 400px;
- height: 400px;
- contain: paint;
-}
.background {
background: lightgreen;
width: 400px;
@@ -26,8 +21,6 @@
}
</style>
-<div id=box>
- <div class=background>
- <div class=item></div>
- </div>
+<div class=background>
+ <div class=item></div>
</div>
```
```
The name of the file: third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-incoming.html
Insertions: 43, Deletions: 0.
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html class=reftest-wait>
+<link rel="help" href="https://github.com/WICG/shared-element-transitions">
+<link rel="author" href="mailto:vmp...@chromium.org">
+<link rel="match" href="root-to-shared-animation-incoming-ref.html">
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+body {
+ background: lightgreen;
+ padding: 0;
+ margin: 0;
+}
+#box {
+ width: 100px;
+ height: 120px;
+ background: blue;
+ contain: paint;
+}
+
+html::page-transition-container(*) { animation-delay: 300s; }
+html::page-transition-incoming-image(*) { animation: unset; opacity: 1; }
+html::page-transition-outgoing-image(*) { animation: unset; opacity: 0; }
+</style>
+
+<div id=box></div>
+
+<script>
+async function runTest() {
+ let t = document.createDocumentTransition();
+ t.start(() => {
+ document.documentElement.style.pageTransitionTag = "none";
+ box.style.pageTransitionTag = "root";
+ // We should not see the "live" body at all.
+ document.body.style.background = "red";
+
+ requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
+ });
+}
+onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
+</script>
+
```
```
The name of the file: third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc
Insertions: 6, Deletions: 6.
@@ -978,8 +978,6 @@
.c_str(),
source_size.Width().ToInt(), source_size.Height().ToInt());
- // TODO(khushalsagar) : The duration/delay in the UA stylesheet will
- // need to be the duration from TransitionConfig. See crbug.com/1275727.
append_selector("html::page-transition-container", tag);
builder.Append("{ animation: page-transition-container-anim-");
builder.Append(tag);
@@ -1040,10 +1038,12 @@
const auto& document_transition_tag = entry.key.GetString();
auto& element_data = entry.value;
- bool tag_is_old_root = old_root_data_ && old_root_data_->tags.Contains(
- document_transition_tag);
- bool tag_is_new_root = new_root_data_ && new_root_data_->tags.Contains(
- document_transition_tag);
+ const bool tag_is_old_root =
+ old_root_data_ &&
+ old_root_data_->tags.Contains(document_transition_tag);
+ const bool tag_is_new_root =
+ new_root_data_ &&
+ new_root_data_->tags.Contains(document_transition_tag);
// The tag can't be both old and new root, since it shouldn't be in the
// `element_data_map_`. This is case 1 above.
DCHECK(!tag_is_old_root || !tag_is_new_root);
```
Approvals:
Vladimir Levin: Commit
Khushal Sagar: Looks good to me
SET: Match root with shared element in animations.
This patch matches root with non-root shared element transitions to
create the animation. There's still some TODOs left about layout size
but that should be handled independently.
R=khusha...@chromium.org
Fixed: 1332576
Change-Id: I4dfc414c95f2ebeba20b0f21df1cea675a8fda0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3722517
Reviewed-by: Khushal Sagar <khusha...@chromium.org>
Commit-Queue: Vladimir Levin <vmp...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1018829}
---
M third_party/blink/renderer/core/document_transition/document_transition_style_tracker.cc
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-end-ref.html
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-end.html
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-incoming-ref.html
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-incoming.html
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-start-ref.html
A third_party/blink/web_tests/wpt_internal/document-transition/root-to-shared-animation-start.html
7 files changed, 372 insertions(+), 71 deletions(-)
Reply all
Reply to author
Forward
0 new messages