Delegated Ink Trails Tracing Improvements [chromium/src : main]
0 views
Skip to first unread message
Olga Gerchikov (Gerrit)
Apr 9, 2026, 3:49:37 PMApr 9
to Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Gaston Rodriguez
Olga Gerchikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Olga Gerchikov
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Olga Gerchikov . resolved
Hi Gaston, can you please review the changes?
Related details
Attention is currently required from:
- Gaston Rodriguez
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gaston Rodriguez (Gerrit)
Apr 14, 2026, 3:46:49 PM (12 days ago) Apr 14
to Olga Gerchikov, chromiu...@luci-project-accounts.iam.gserviceaccount.com, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Olga Gerchikov
Gaston Rodriguez added 3 comments![]( "Open in Gerrit")
File content/browser/renderer_host/render_widget_host_impl.cc
Line 2640, Patchset 2 (Latest):
aura::Window* root_window =
view_->GetNativeView()->GetRootWindow();
if (root_window) {Gaston Rodriguez . unresolved
```suggestion
if (aura::Window* root_window =
view_->GetNativeView()->GetRootWindow()) {
```
I think you may be able to at least declare the pointer as `const`.
Line 2649, Patchset 2 (Latest):
}
#endif
ctx.AddDebugAnnotation("screen point", screen_point.ToString());
});Gaston Rodriguez . unresolved
```suggestion
}
ctx.AddDebugAnnotation("screen point", screen_point.ToString());
#endif
});
```
I think you should only add "screen point" on Windows unless there's a reason to provide the same "interface" or data for all platforms, in which case this is fine.
In other platforms, "screen point" is redundant with "delegated point".
I suspect it's not true that `delegated_ink_point.point()` is the screen point in other platforms.
File ui/gl/delegated_ink_point_renderer_gpu.cc
Line 135, Patchset 2 (Latest):
TRACE_EVENT("delegated_ink_trails",Gaston Rodriguez . unresolved
I'm not sure I understand what information this trace would provide.
For each point, how long it takes to set their painted timestamp if they aren't already in the `token_map`?
Related details
Attention is currently required from:
- Olga Gerchikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Gerchikov (Gerrit)
Apr 21, 2026, 11:26:08 PM (5 days ago) Apr 21
to Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Gaston Rodriguez
Olga Gerchikov added 3 comments![]( "Open in Gerrit")
File content/browser/renderer_host/render_widget_host_impl.cc
Line 2640, Patchset 2:
aura::Window* root_window =
view_->GetNativeView()->GetRootWindow();
if (root_window) {Gaston Rodriguez . resolved
```suggestion
if (aura::Window* root_window =
view_->GetNativeView()->GetRootWindow()) {
```
I think you may be able to at least declare the pointer as `const`.
Olga Gerchikov
It can't be const:
../../content/browser/renderer_host/render_widget_host_impl.cc(2643,29): error: 'this' argument to member function 'GetAcceleratedWidget' has type 'const WindowTreeHost', but function is not marked const
2643 | const HWND hwnd = root_window->GetHost()->GetAcceleratedWidget();
#endif
ctx.AddDebugAnnotation("screen point", screen_point.ToString());
});Gaston Rodriguez . resolved
```suggestion
}
ctx.AddDebugAnnotation("screen point", screen_point.ToString());#endif
});
```
I think you should only add "screen point" on Windows unless there's a reason to provide the same "interface" or data for all platforms, in which case this is fine.In other platforms, "screen point" is redundant with "delegated point".
I suspect it's not true that `delegated_ink_point.point()` is the screen point in other platforms.
Olga Gerchikov
Done
File ui/gl/delegated_ink_point_renderer_gpu.cc
I'm not sure I understand what information this trace would provide.
For each point, how long it takes to set their painted timestamp if they aren't already in the `token_map`?
Olga Gerchikov
This is called right after commit and indicates when the point is guaranteed to be drawen.
Related details
Attention is currently required from:
- Gaston Rodriguez
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gaston Rodriguez (Gerrit)
Apr 22, 2026, 11:06:44 AM (4 days ago) Apr 22
to Olga Gerchikov, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Olga Gerchikov
Gaston Rodriguez added 1 comment![]( "Open in Gerrit")
File ui/gl/delegated_ink_point_renderer_gpu.cc
Line 135, Patchset 2:
TRACE_EVENT("delegated_ink_trails",Gaston Rodriguez . resolved
Olga GerchikovI'm not sure I understand what information this trace would provide.
For each point, how long it takes to set their painted timestamp if they aren't already in the `token_map`?
This is called right after commit and indicates when the point is guaranteed to be drawen.
Gaston Rodriguez
I see, it's not about the duration but reporting which point was painted and having the flow in the ui to track a single point.
I was going to suggest extracting the trace event from the `for`, but I think that this is the better solution.
Related details
Attention is currently required from:
- Olga Gerchikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gaston Rodriguez (Gerrit)
Apr 22, 2026, 11:06:56 AM (4 days ago) Apr 22
to Olga Gerchikov, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Olga Gerchikov
Gaston Rodriguez voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Olga Gerchikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Gerchikov (Gerrit)
Apr 22, 2026, 12:17:20 PM (4 days ago) Apr 22
to Avi Drissman, Mikhail Khokhlov, Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Avi Drissman and Mikhail Khokhlov
Olga Gerchikov added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Olga Gerchikov . resolved
Hi, can you please review the change? Thanks!
Related details
Attention is currently required from:
- Avi Drissman
- Mikhail Khokhlov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mikhail Khokhlov (Gerrit)
Apr 22, 2026, 12:28:39 PM (4 days ago) Apr 22
to Olga Gerchikov, Avi Drissman, Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Avi Drissman and Olga Gerchikov
Mikhail Khokhlov voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Avi Drissman
- Olga Gerchikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Avi Drissman (Gerrit)
Apr 22, 2026, 12:31:20 PM (4 days ago) Apr 22
to Olga Gerchikov, Avi Drissman, Mikhail Khokhlov, Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Olga Gerchikov
Avi Drissman voted and added 1 comment![]( "Open in Gerrit")
Votes added by Avi Drissman
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Olga Gerchikov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olga Gerchikov (Gerrit)
Apr 22, 2026, 1:03:26 PM (4 days ago) Apr 22
to Avi Drissman, Mikhail Khokhlov, Gaston Rodriguez, Chromium LUCI CQ, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Olga Gerchikov voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Apr 22, 2026, 1:17:32 PM (4 days ago) Apr 22
to Olga Gerchikov, Avi Drissman, Mikhail Khokhlov, Gaston Rodriguez, chromium...@chromium.org, alexmo...@chromium.org, creis...@chromium.org, navigation...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Delegated Ink Trails Tracing Improvements
This change enables end‑to‑end collection of delegated ink traces on the
Windows platform.
Updates include:
- Added the delegated_ink_trails category to ETW events.
- Emitted delegated ink points in screen coordinates from RenderWidgetHostImpl::ForwardDelegatedInkPoint.
- Added tracing immediately after frame commit.
Change-Id: I2628e574988864b92b5b9b210bb89afc8ab6ed64
Reviewed-by: Gaston Rodriguez <gas...@microsoft.com>
Reviewed-by: Avi Drissman <a...@chromium.org>
Commit-Queue: Olga Gerchikov <gerc...@microsoft.com>
Reviewed-by: Mikhail Khokhlov <khok...@google.com>
Cr-Commit-Position: refs/heads/main@{#1618947}
Files:
- M base/trace_event/etw_interceptor_win.cc
- M content/browser/renderer_host/render_widget_host_impl.cc
- M ui/gl/delegated_ink_point_renderer_gpu.cc
Change size: S
Delta: 3 files changed, 35 insertions(+), 5 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Gaston Rodriguez, +1 by Mikhail Khokhlov, +1 by Avi Drissman
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages