This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[fuchsia] Remove extraneous ShapeNodes #10150
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mklim
reviewed
Jul 25, 2019
|
|
||
| // Helper function to generate clip planes for a scenic::EntityNode. | ||
| static void SetEntityNodeClipPlanes(scenic::EntityNode* entity_node, | ||
| static void SetEntityNodeClipPlanes(scenic::EntityNode& entity_node, |
Contributor
Author
There was a problem hiding this comment.
Ultimately, that EntityNode is moved from so it can't be const
liyuqian
approved these changes
Jul 26, 2019
Contributor
liyuqian
left a comment
liyuqian
left a comment
There was a problem hiding this comment.
LGTM except a little concern about testing.
As Michael pointed out, we don't have any test to cover this in the engine repo. Hence can you please describe your manual test method to the commit/PR description (including how to verify that ShapeNodes count is reduced in Fuchsia)? It will allow us to debug this if something goes wrong unexpectedly.
flow/scene_update_context.cc
Outdated
| SceneUpdateContext::Frame::~Frame() { | ||
| context().CreateFrame(std::move(entity_node_ptr()), | ||
| std::move(shape_node_ptr()), rrect_, color_, | ||
| context().CreateFrame(std::move(entity_node()), |
Contributor
There was a problem hiding this comment.
Has https://fuchsia.atlassian.net/browse/SCN-268 been resolved? If so, I'm very glad that we can finally move the node directly without using so many pointers :)
Contributor
Author
There was a problem hiding this comment.
Yep, it has been resolved :)
Test: Ran test programs on Fuchsia and dumped Scenic scene graph using `fx shell cat /hub/c/scenic.cmx/*/out/debug/dump-scenes` Inspected scene graph to confirm removal of ShapeNodes. FL-284 #done
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 31, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 31, 2019
This was referenced Jul 31, 2019
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 31, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Jul 31, 2019
flutter/engine@b43178e...a29ef66 git log b43178e..a29ef66 --no-merges --oneline a29ef66 [fuchsia] Remove extraneous ShapeNodes (flutter/engine#10150) 8ef792f Roll src/third_party/skia a13c7744f154f434b5415688280b4e127a2ce8f5..6808e2d1faaccd6fc739f436c2470f199aa4d1a8 (2 commits) (flutter/engine#10281) ca29c72 Roll fuchsia/sdk/core/mac-amd64 from fuchsia/sdk/core/mac-amd64:vjX_DhfQqJjsX26pl5-vNNM3Lfb0Vwl9l09abYj-D1oC to fuchsia/sdk/core/mac-amd64:A4Ah4g0K30dWnbolQ7Z3zyXapbAyy3d9l4IeLGAG9YQC (flutter/engine#10280) 75970d7 Roll fuchsia/sdk/core/linux-amd64 from fuchsia/sdk/core/linux-amd64:om5F-s2gIabc-Toen2VT6nnosnQvTKI1e6Nz2zg8P40C to fuchsia/sdk/core/linux-amd64:6j9WXOOtqKZzSKIgQi-12GJ1MYzPDJvq-RLAtpJpGB0C (flutter/engine#10279) 6a69cd5 Roll src/third_party/skia 6980c4e0aef8ab9546e25d8160c05a716f1c3ce3..a13c7744f154f434b5415688280b4e127a2ce8f5 (1 commits) (flutter/engine#10278) 7a7afab Roll fuchsia/sdk/core/linux-amd64 from fuchsia/sdk/core/linux-amd64:MuVYOXXS9sxCbxvFCJDHfuF5pkWXIgS0uUlBFdIuQPsC to fuchsia/sdk/core/linux-amd64:om5F-s2gIabc-Toen2VT6nnosnQvTKI1e6Nz2zg8P40C (flutter/engine#10277) 02649bd Roll fuchsia/sdk/core/mac-amd64 from fuchsia/sdk/core/mac-amd64:kSzcB40iCqsXbCka_VUf4jB5iRl2G9gLM_xDbbGg5GwC to fuchsia/sdk/core/mac-amd64:vjX_DhfQqJjsX26pl5-vNNM3Lfb0Vwl9l09abYj-D1oC (flutter/engine#10276) e8b19de Roll src/third_party/skia 6e86c1b05eeedc3b6be0e98001b7b5d6bab71b74..6980c4e0aef8ab9546e25d8160c05a716f1c3ce3 (4 commits) (flutter/engine#10275) 3be7523 Roll fuchsia/sdk/core/linux-amd64 from fuchsia/sdk/core/linux-amd64:pGTb76UZ-PsJzdatSNkJIkEGLESaGa_S2wtZfgHPU5EC to fuchsia/sdk/core/linux-amd64:MuVYOXXS9sxCbxvFCJDHfuF5pkWXIgS0uUlBFdIuQPsC (flutter/engine#10274) ed8e35c Remove get engine (flutter/engine#9747) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These were always being generated, even when not being used.
closes flutter/flutter#36766