PaintWorklet: create a PaintWorklet 'bundle' structure and pass it from main to cc |
||||
Issue descriptionAs noted in the design doc (https://docs.google.com/document/d/1USTH2Vd4D2tALsvZvy4B2aWotKWjkCYP5m0g7b90RAU/view), this structure must capture the following information to be passed from main to cc/ : * 'paint function' - the JavaScript callback provided by the developer * 'paint size' - the size of the target at paint time * 'input properties' - a map of the input properties for the CSS Paint instance, to the current value for those properties at paint time. For the first milestone we will ignore cc/ animations, so 'input properties' will always just contain the most recent main-computed value and cc/ will use that. However in the future we will require some way to track which properties currently have a cc/ animation (so thought should be given to that). The hardest part to store is the paint function, since cc/ obviously cannot refer to v8/javascript directly. Inspiration can probably be taken from AnimationWorklet's approach: cc/ defines a cc::LayerTreeMutator class (https://cs.chromium.org/chromium/src/cc/trees/layer_tree_mutator.h?l=143&rcl=c300e9a9dc86db06e3ceb156daef990a17012ef8), which is implemented by blink::CompositorMutatorClient (https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/compositor_mutator_client.h?l=17&rcl=c300e9a9dc86db06e3ceb156daef990a17012ef8) The CSS paint version of blink::CompositorMutatorClient would likely wrap a blink::CSSPaintDefinition for now, but there may be further indirection needed (can platform/graphics depend on modules/ for a start? It looks like for AnimationWorklet a 'AnimationWorkletMutator' class is defined in platform/graphics and implemented in modules/animationworklet, we would probably need to do the same?) To aid with de-duping PaintWorklets, we should probably define hash and equality on these bundles. I have not yet determined how we would check for equality of registered paint callback (probably just store the name, is that unique?)
,
Sep 17
Note: this bug also tracks the work of having main create the bundle, without replacing the DisplayItems on main-thread or passing the bundle to cc/. Doing that integration makes sure that the creation works in real code (probably flag-hidden) and doesn't stop working if someone changes the rest of the PaintWorklet code.
,
Sep 25
,
Oct 1
Note: the prototype CL has much of this work done: https://chromium-review.googlesource.com/c/chromium/src/+/1251465
,
Oct 1
,
Oct 1
Specific pointers from the milestone CL. The bundle structure: https://chromium-review.googlesource.com/c/chromium/src/+/1251465/7/cc/trees/layer_tree_painter.h Creating the bundle structure: https://chromium-review.googlesource.com/c/chromium/src/+/1251465/7/third_party/blink/renderer/core/css/css_paint_value.cc Stored on Document (bad!): https://chromium-review.googlesource.com/c/chromium/src/+/1251465/7/third_party/blink/renderer/core/dom/document.h Passed to cc: https://chromium-review.googlesource.com/c/chromium/src/+/1251465/7/third_party/blink/renderer/core/dom/document.h
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1 commit fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1 Author: Xida Chen <xidachen@chromium.org> Date: Tue Oct 16 16:32:21 2018 Add a PaintWorkletInput structure This CL adds a new PaintWorkletInput structure which includes some necessary information that is needed for PaintWorklet to paint. Bug: 884698 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic6f4960c383191c33e1bbfbc18a1d3f7826b3ef2 Reviewed-on: https://chromium-review.googlesource.com/c/1256371 Reviewed-by: David Bokan <bokan@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#600012} [modify] https://crrev.com/fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1/cc/BUILD.gn [add] https://crrev.com/fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1/cc/trees/layer_tree_painter.h [modify] https://crrev.com/fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1/third_party/blink/renderer/platform/BUILD.gn [add] https://crrev.com/fe9b2ac001d75a06b25ed3201aa2afbdd4f7b1a1/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/705775845f6e50a89bc4bad7a2a18126d936c655 commit 705775845f6e50a89bc4bad7a2a18126d936c655 Author: Xida Chen <xidachen@chromium.org> Date: Tue Oct 23 22:28:06 2018 Make PlatformPaintWorkletInput take a FloatSize Right now the CSSPaintDefinition::Paint takes a FloatSize which is the container size without subpixel snapping. So when we create the paint worklet input, we should use the same FloatSize. Bug: 884698 Change-Id: Ifcabb84cdb8db041a8ebd28112d4943c4c7adcb0 Reviewed-on: https://chromium-review.googlesource.com/c/1291783 Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#602137} [modify] https://crrev.com/705775845f6e50a89bc4bad7a2a18126d936c655/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/705775845f6e50a89bc4bad7a2a18126d936c655 commit 705775845f6e50a89bc4bad7a2a18126d936c655 Author: Xida Chen <xidachen@chromium.org> Date: Tue Oct 23 22:28:06 2018 Make PlatformPaintWorkletInput take a FloatSize Right now the CSSPaintDefinition::Paint takes a FloatSize which is the container size without subpixel snapping. So when we create the paint worklet input, we should use the same FloatSize. Bug: 884698 Change-Id: Ifcabb84cdb8db041a8ebd28112d4943c4c7adcb0 Reviewed-on: https://chromium-review.googlesource.com/c/1291783 Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#602137} [modify] https://crrev.com/705775845f6e50a89bc4bad7a2a18126d936c655/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bcd072a3785500e899e931119e94cba62376fae5 commit bcd072a3785500e899e931119e94cba62376fae5 Author: Xida Chen <xidachen@chromium.org> Date: Wed Dec 19 12:53:46 2018 Create a PaintWorkletStylePropertyMap In the current CSSPaintDefinition::Paint which runs on the main thread, it creates a PrepopulatedStylePropertyMap. That style map contains some information that cannot be passed cross thread. This CL creates a PaintWorkletStylePropertyMap, it is designed to be built from the worklet thread which can be passed to the JS paint callback. What will typically happen is that on the main thread, in the CSSPaintValue::GetImage, we will create a PaintWorkletInput which contains the thread-safe css properties (PaintWorkletStylePropertyMap). PaintWorkletStylePropertyMap can be be passed to a worklet thread and used by the javascript paint callback. Once the Blink::PaintWorkletInput is passed to the worklet thread, we then build a PaintWorkletStylePropertyMap based on that input, and give this style map to the JS paint callback. We have added a test to make sure that the PaintWorkletInput can be passed cross thread and that no information is lost. Bug: 884698 Change-Id: Ie576e98d142d5f8ca6a09291cd7b0897f976817c Reviewed-on: https://chromium-review.googlesource.com/c/1281702 Commit-Queue: Xida Chen <xidachen@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#617810} [modify] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/DEPS [modify] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/BUILD.gn [modify] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/css_unsupported_style_value.h [add] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/paint_worklet_input.cc [add] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/paint_worklet_input.h [add] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.cc [add] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.h [add] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map_test.cc [modify] https://crrev.com/bcd072a3785500e899e931119e94cba62376fae5/third_party/blink/renderer/platform/BUILD.gn [delete] https://crrev.com/47fa6c3e4f59cf7c80cf7db72c64a16fe66bc9d2/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f36077b6090c6837a799765aa96bfb64f543a13 commit 1f36077b6090c6837a799765aa96bfb64f543a13 Author: Xida Chen <xidachen@chromium.org> Date: Wed Dec 19 19:08:35 2018 Revert "Create a PaintWorkletStylePropertyMap" This reverts commit bcd072a3785500e899e931119e94cba62376fae5. Reason for revert: I am reverting the "delete CSSPropertyID hash trait" CL: https://chromium-review.googlesource.com/c/chromium/src/+/1384924 because of a crash. So I will revert this CL too. The solution will be keeping one hashmap from String to String as Rune suggested. Original change's description: > Create a PaintWorkletStylePropertyMap > > In the current CSSPaintDefinition::Paint which runs on the main thread, > it creates a PrepopulatedStylePropertyMap. That style map contains > some information that cannot be passed cross thread. > > This CL creates a PaintWorkletStylePropertyMap, it is designed to be > built from the worklet thread which can be passed to the JS paint > callback. > > What will typically happen is that on the main thread, in > the CSSPaintValue::GetImage, we will create a PaintWorkletInput > which contains the thread-safe css properties > (PaintWorkletStylePropertyMap). PaintWorkletStylePropertyMap > can be be passed to a worklet thread and used by the javascript > paint callback. > > Once the Blink::PaintWorkletInput is passed to the worklet thread, > we then build a PaintWorkletStylePropertyMap based on that input, and > give this style map to the JS paint callback. > > We have added a test to make sure that the PaintWorkletInput > can be passed cross thread and that no information is lost. > > Bug: 884698 > > Change-Id: Ie576e98d142d5f8ca6a09291cd7b0897f976817c > Reviewed-on: https://chromium-review.googlesource.com/c/1281702 > Commit-Queue: Xida Chen <xidachen@chromium.org> > Reviewed-by: Philip Rogers <pdr@chromium.org> > Reviewed-by: Rune Lillesveen <futhark@chromium.org> > Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> > Cr-Commit-Position: refs/heads/master@{#617810} TBR=pdr@chromium.org,xidachen@chromium.org,smcgruer@chromium.org,futhark@chromium.org,andruud@chromium.org Change-Id: I3f53970dbbd19cd401262411bdabcb0f1c3ad912 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 884698 Reviewed-on: https://chromium-review.googlesource.com/c/1384846 Reviewed-by: Xida Chen <xidachen@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#617900} [modify] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/core/DEPS [modify] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/core/css/BUILD.gn [modify] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/core/css/cssom/css_unsupported_style_value.h [delete] https://crrev.com/47a2af4a2b146fff146d826c91a88f9d7afd1bb3/third_party/blink/renderer/core/css/cssom/paint_worklet_input.cc [delete] https://crrev.com/47a2af4a2b146fff146d826c91a88f9d7afd1bb3/third_party/blink/renderer/core/css/cssom/paint_worklet_input.h [delete] https://crrev.com/47a2af4a2b146fff146d826c91a88f9d7afd1bb3/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.cc [delete] https://crrev.com/47a2af4a2b146fff146d826c91a88f9d7afd1bb3/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.h [delete] https://crrev.com/47a2af4a2b146fff146d826c91a88f9d7afd1bb3/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map_test.cc [modify] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/platform/BUILD.gn [add] https://crrev.com/1f36077b6090c6837a799765aa96bfb64f543a13/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Dec 19
In the patch that deleted CSSPropertyID hash trait, you said you found a bug. Instead of working around that bug, can it be fixed?
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fabee74ba66a490bfdb6014bec5a9f2094a93a9e commit fabee74ba66a490bfdb6014bec5a9f2094a93a9e Author: Xida Chen <xidachen@chromium.org> Date: Thu Dec 20 21:31:43 2018 Reland: Create a PaintWorkletStylePropertyMap This CL is a re-land of: https://chromium-review.googlesource.com/c/chromium/src/+/1281702 Previous CL depends on the usage of default hash traits implementation of enum, which could crash. In this CL, instead of keeping two hash maps: 1. HashMap<CSSPropertyID, String> native_values_ 2. HashMap<String, String> custom_values_ We keep one hash map only: HashMap<String, String> values_ Because we guarantee that CSS native and custom property name will never conflict, this is safe. PS#1 is the original CL so that we can easily see the diffs. TBR=pdr@chromium.org Bug: 884698 Change-Id: Ifc74a13ba67fa74ff1a3dd9dbca628675b42c777 Reviewed-on: https://chromium-review.googlesource.com/c/1386648 Commit-Queue: Xida Chen <xidachen@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#618328} [modify] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/DEPS [modify] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/BUILD.gn [modify] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/css_unsupported_style_value.h [add] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/paint_worklet_input.cc [add] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/paint_worklet_input.h [add] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.cc [add] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map.h [add] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/core/css/cssom/paint_worklet_style_property_map_test.cc [modify] https://crrev.com/fabee74ba66a490bfdb6014bec5a9f2094a93a9e/third_party/blink/renderer/platform/BUILD.gn [delete] https://crrev.com/8defb9b5b1cc44033c22761a21a5fec392f430c9/third_party/blink/renderer/platform/graphics/platform_paint_worklet_input.h
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49ac5d53f5192ae2677b9c4e975f5e7b55effef7 commit 49ac5d53f5192ae2677b9c4e975f5e7b55effef7 Author: Xida Chen <xidachen@chromium.org> Date: Fri Dec 21 20:56:27 2018 [Code health] Make some members in Blink::PaintWorkletInput const The Blink::PaintWorkletInput is a const data structure, once it is created, its member should never be changed. Bug: 884698 Change-Id: I9ac31481066fb6a6f2f0d02524b4136f3f084d23 Reviewed-on: https://chromium-review.googlesource.com/c/1388108 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#618592} [modify] https://crrev.com/49ac5d53f5192ae2677b9c4e975f5e7b55effef7/third_party/blink/renderer/core/css/cssom/paint_worklet_input.cc [modify] https://crrev.com/49ac5d53f5192ae2677b9c4e975f5e7b55effef7/third_party/blink/renderer/core/css/cssom/paint_worklet_input.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by smcgruer@chromium.org
, Sep 17