Implements the naming of untracked gles handles #56948
Conversation
gaaclarke
force-pushed
the
name-untracked-handles
branch
2 times, most recently
from
fa7f8bf to
d7eaf82
Compare
gaaclarke
force-pushed
the
name-untracked-handles
branch
2 times, most recently
from
dbe414f to
11189f6
Compare
gaaclarke
force-pushed
the
name-untracked-handles
branch
from
11189f6 to
d98c37e
Compare
| handle.untracked_id_.value(), std::string(label))); | ||
| } else { | ||
| if (auto found = handles_.find(handle); found != handles_.end()) { | ||
| found->second.pending_debug_label = label; |
There was a problem hiding this comment.
We can't just store this in handles_to_name_ right now since the thing might not have been created yet.
There was a problem hiding this comment.
If the untracked handles are created immediately, why do we need to go through the reactor managed labeling at all? Can we use a different labeling function that calls the debug label methods directly?
There was a problem hiding this comment.
We can and I originally implemented it that way. I was thinking these would just get called once in a while so we might as well keep the same semantics, but there are things that we create each frame isn't there? So maybe I should switch it back.
There was a problem hiding this comment.
We do a lot of a labeling. it should be mostly switched off in release mode but it will still get in the way for profile mode.
There was a problem hiding this comment.
FWIW I'm not sure where we will land on this. I am seeing code trying to create DeviceBufferGLES from threads that can't react. That's weird to me since we store ops based on the thread. I'm still sorting through this. This is good for now though.
gaaclarke
added
the
autosubmit
| FML_DCHECK(CanReactOnCurrentThread()); | ||
| const auto& gl = GetProcTable(); | ||
| gl.SetDebugLabel(ToDebugResourceType(handle.GetType()), | ||
| handle.untracked_id_.value(), std::string(label)); |
There was a problem hiding this comment.
oh wait, can we make this function use a string_view? Right now its using a const string ref, so it should be free to switch it out.
gaaclarke
removed
the
autosubmit
gaaclarke
added
the
autosubmit
…159836) flutter/engine@8d3c718...05e2d65 2024-12-05 jonahwilliams@google.com [Impeller] store GLES bindings on render pass w/ offsets instead of per-command. (flutter/engine#56910) 2024-12-05 jonahwilliams@google.com [Impeller] avoid re-binding winding order and cull mode. (flutter/engine#56943) 2024-12-05 30870216+gaaclarke@users.noreply.github.com Implements the naming of untracked gles handles (flutter/engine#56948) 2024-12-05 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from VilXq4eGH5A24wRWA... to r9Dc5VRF6sE3pJH20... (flutter/engine#56953) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC bdero@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
issue: flutter#159745 flutter/engine#56927 introduced untracked handles, but naming them didn't work. This adds a test to make sure they work. I kept naming thread-safe since it isn't happening often anyways. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
2 participants
issue: flutter/flutter#159745
#56927 introduced untracked handles, but naming them didn't work. This adds a test to make sure they work. I kept naming thread-safe since it isn't happening often anyways.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.