Fix OffscreenCanvas::PushFrame damage rect [chromium/src : main]
Vasiliy Telezhnikov (Gerrit)
Vasiliy Telezhnikov added 1 comment![]( "Open in Gerrit")
Please, take a look.
Ignore the CL chain, I likely will abandon them, I don't like where it ending up to be and will go the opposite direction.
Related details
- Colin Blundell
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Colin Blundell voted and added 2 comments![]( "Open in Gerrit")
Votes added by Colin Blundell
| Code-Review | +1 |
2 comments
Thanks!
check is incorrect and we should remove it.This is a doozy. Am I understanding correctly that this means that WebGL won't actually draw to the screen for offscreen canvas in cases where the drawing buffer size is smaller than the canvas size?
I also don't understand how OffscreenCanvas2D works [at all](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc;l=422;drc=23c1350a5eb632830a9ece307987fdd6816893f1;bpv=1;bpt=1) - am I missing something there?
Related details
- Vasiliy Telezhnikov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Colin Blundell added 1 comment![]( "Open in Gerrit")
check is incorrect and we should remove it.This is a doozy. Am I understanding correctly that this means that WebGL won't actually draw to the screen for offscreen canvas in cases where the drawing buffer size is smaller than the canvas size?
I also don't understand how OffscreenCanvas2D works [at all](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc;l=422;drc=23c1350a5eb632830a9ece307987fdd6816893f1;bpv=1;bpt=1) - am I missing something there?
Actually, pursuing this further, this CL is [*not*](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc;l=1465-1479?q=DrawingBuffer::Ad&ss=chromium) a no-op for WebGL, is it? We'll now draw to the screen in cases where we wouldn't before. It's not clear to me what happens for the pixels not covered by the drawing buffer? (This overlaps with my workstream for marking external writes as complete overwrites).
Related details
- Vasiliy Telezhnikov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasiliy Telezhnikov (Gerrit)
Vasiliy Telezhnikov added 1 comment![]( "Open in Gerrit")
check is incorrect and we should remove it.Colin BlundellThis is a doozy. Am I understanding correctly that this means that WebGL won't actually draw to the screen for offscreen canvas in cases where the drawing buffer size is smaller than the canvas size?
I also don't understand how OffscreenCanvas2D works [at all](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc;l=422;drc=23c1350a5eb632830a9ece307987fdd6816893f1;bpv=1;bpt=1) - am I missing something there?
Actually, pursuing this further, this CL is [*not*](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc;l=1465-1479?q=DrawingBuffer::Ad&ss=chromium) a no-op for WebGL, is it? We'll now draw to the screen in cases where we wouldn't before. It's not clear to me what happens for the pixels not covered by the drawing buffer? (This overlaps with my workstream for marking external writes as complete overwrites).
This CL doesn't change anything related to Dispatcher size or resource size, it just changes the damage rect that we pass to display compositor. So nothing in regard of this size check changes, or I misunderstood your question?
Yes, WebGL and ImageBitmap canvas right now don't present anything if size mismatches.
There is dispatcher size, which becomes render pass / compositor frame size. There is resource size, which becomes transferable resource size.
And there is damage rect, a hint that tells display compositor which portion of the render pass changed since the last frame. This was in a resource space, now it's in a render pass space as it supposed to be. Normally it wouldn't be no-op, but of that size check we do't submit frames in that case anyway.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Colin Blundell added 1 comment![]( "Open in Gerrit")
check is incorrect and we should remove it.Colin BlundellThis is a doozy. Am I understanding correctly that this means that WebGL won't actually draw to the screen for offscreen canvas in cases where the drawing buffer size is smaller than the canvas size?
I also don't understand how OffscreenCanvas2D works [at all](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc;l=422;drc=23c1350a5eb632830a9ece307987fdd6816893f1;bpv=1;bpt=1) - am I missing something there?
Vasiliy TelezhnikovActually, pursuing this further, this CL is [*not*](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc;l=1465-1479?q=DrawingBuffer::Ad&ss=chromium) a no-op for WebGL, is it? We'll now draw to the screen in cases where we wouldn't before. It's not clear to me what happens for the pixels not covered by the drawing buffer? (This overlaps with my workstream for marking external writes as complete overwrites).
This CL doesn't change anything related to Dispatcher size or resource size, it just changes the damage rect that we pass to display compositor. So nothing in regard of this size check changes, or I misunderstood your question?
Yes, WebGL and ImageBitmap canvas right now don't present anything if size mismatches.
There is dispatcher size, which becomes render pass / compositor frame size. There is resource size, which becomes transferable resource size.
And there is damage rect, a hint that tells display compositor which portion of the render pass changed since the last frame. This was in a resource space, now it's in a render pass space as it supposed to be. Normally it wouldn't be no-op, but of that size check we do't submit frames in that case anyway.
That makes sense, thanks! You understood my question correctly - I got the damage rect and the resource size conflated.
Related details
- Vasiliy Telezhnikov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vasiliy Telezhnikov (Gerrit)
Vasiliy Telezhnikov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Vasiliy Telezhnikov
| Commit-Queue | +2 |
1 comment
Thanks for the review.
Related details
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)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Fix OffscreenCanvas::PushFrame damage rect
All non-2d cases don't really track damage rect and want to damage whole
canvas every frame.
The damage rect is supposed to be in the render pass space, not the
texture space, so we need to use canvas size and not shared image size.
To simplify further refactoring, we make damage rect optional and make
full damage if it's nullopt.
Note, it's no-op because of the image size check here [1], but that
check is incorrect and we should remove it.
- M third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h
- M third_party/blink/renderer/core/html/canvas/html_canvas_element.cc
- M third_party/blink/renderer/core/html/canvas/html_canvas_element.h
- M third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc
- M third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h
- M third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_rendering_context.cc
- M third_party/blink/renderer/modules/canvas/offscreencanvas/offscreen_canvas_test.cc
- M third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
- M third_party/blink/renderer/modules/webgpu/gpu_canvas_context.cc
Code-Review: +1 by Colin Blundell
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |