WebXR Layers: Fix clearOnAccess [chromium/src : main]
Alexander Cooper (Gerrit)
Alexander Cooper added 4 comments![]( "Open in Gerrit")
I'm a bit confused on the timing here, we're not clearing it on access we're clearing it at the end of access?
OnCurrentTextureProduced();Maybe `OnTextureAccessed()` ? And maybe it should happen outside of the if? Otherwise, I think we aren't always clearing it?
// clear the old buffer.Can you elaborate? Or maybe just indicate that it's because it'll handle the clearing? Even if it does, any harm in storing it?
if (descriptor().clear_on_access) {Doesn't seem right? It may not have been accessed here?
Related details
- Brandon Jones
- Evgenii Alekseev (xWF)
- Yong Li (xWF)
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. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 3 comments![]( "Open in Gerrit")
OnCurrentTextureProduced();Maybe `OnTextureAccessed()` ? And maybe it should happen outside of the if? Otherwise, I think we aren't always clearing it?
This is for the shared image swapchain. In that case the texture is reset after each frame. Moving it out of the if will make it called too frequently
// clear the old buffer.Can you elaborate? Or maybe just indicate that it's because it'll handle the clearing? Even if it does, any harm in storing it?
The texture array wrapper will clear the buffer. The original swap chain will always be fully rewritten from the wrapper. This is only about performance.
if (descriptor().clear_on_access) {Doesn't seem right? It may not have been accessed here?
Yeah, I do see the currently solution is confusing. Basically this CL uses different strategy for shared image swapchain and others.
I will try to unify the 2 solutions.
Related details
- Alexander Cooper
- Brandon Jones
- Evgenii Alekseev (xWF)
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. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 2 comments![]( "Open in Gerrit")
Yong Li (xWF)Maybe `OnTextureAccessed()` ? And maybe it should happen outside of the if? Otherwise, I think we aren't always clearing it?
This is for the shared image swapchain. In that case the texture is reset after each frame. Moving it out of the if will make it called too frequently
Got a new solution
Yong Li (xWF)Doesn't seem right? It may not have been accessed here?
Yeah, I do see the currently solution is confusing. Basically this CL uses different strategy for shared image swapchain and others.
I will try to unify the 2 solutions.
Done
Alexander Cooper (Gerrit)
Alexander Cooper voted and added 3 comments![]( "Open in Gerrit")
Votes added by Alexander Cooper
| Code-Review | +1 |
3 comments
// clear the old buffer.Yong Li (xWF)Can you elaborate? Or maybe just indicate that it's because it'll handle the clearing? Even if it does, any harm in storing it?
The texture array wrapper will clear the buffer. The original swap chain will always be fully rewritten from the wrapper. This is only about performance.
Will this boolean be used if the texture type is texture array? If not, maybe worth just setting it anyway.
// Copy the wrapped swap chain's descriptor and divide its width by the
// number of requested layers.outdated comment
Related details
- Brandon Jones
- Evgenii Alekseev (xWF)
- Yong Li (xWF)
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. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
// clear the old buffer.Yong Li (xWF)Can you elaborate? Or maybe just indicate that it's because it'll handle the clearing? Even if it does, any harm in storing it?
Alexander CooperThe texture array wrapper will clear the buffer. The original swap chain will always be fully rewritten from the wrapper. This is only about performance.
Will this boolean be used if the texture type is texture array? If not, maybe worth just setting it anyway.
the texture array swapchain will set the clear_on_access to init->clearOnAccess(). I will update the comments to make it more clear.
Related details
- Alexander Cooper
- Brandon Jones
- Evgenii Alekseev (xWF)
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. |
Alexander Cooper (Gerrit)
Alexander Cooper voted and added 2 comments![]( "Open in Gerrit")
Votes added by Alexander Cooper
| Code-Review | +1 |
2 comments
// clear the old buffer.Yong Li (xWF)Can you elaborate? Or maybe just indicate that it's because it'll handle the clearing? Even if it does, any harm in storing it?
Alexander CooperThe texture array wrapper will clear the buffer. The original swap chain will always be fully rewritten from the wrapper. This is only about performance.
Yong Li (xWF)Will this boolean be used if the texture type is texture array? If not, maybe worth just setting it anyway.
the texture array swapchain will set the clear_on_access to init->clearOnAccess(). I will update the comments to make it more clear.
Done
// Copy the wrapped swap chain's descriptor and divide its width by the
// number of requested layers.- Brandon Jones
- Evgenii Alekseev (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Jones (Gerrit)
Brandon Jones voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brandon Jones
| Code-Review | +1 |
1 comment
Related details
- Evgenii Alekseev (xWF)
- Yong Li (xWF)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
Yong Li (xWF) voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Evgenii Alekseev (xWF)
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
WebXR Layers: Fix clearOnAccess
Clear buffers only when clearOnAccess is true.
- M third_party/blink/renderer/modules/xr/xr_layer_init.idl
- M third_party/blink/renderer/modules/xr/xr_swap_chain.h
- M third_party/blink/renderer/modules/xr/xr_webgl_binding.cc
- M third_party/blink/renderer/modules/xr/xr_webgl_binding.h
- M third_party/blink/renderer/modules/xr/xr_webgl_cubemap_swap_chain.cc
- M third_party/blink/renderer/modules/xr/xr_webgl_cubemap_swap_chain.h
- M third_party/blink/renderer/modules/xr/xr_webgl_swap_chain.cc
- M third_party/blink/renderer/modules/xr/xr_webgl_swap_chain.h
- M third_party/blink/renderer/modules/xr/xr_webgl_texture_array_swap_chain.cc
- M third_party/blink/renderer/modules/xr/xr_webgl_texture_array_swap_chain.h
Code-Review: +1 by Brandon Jones, +1 by Alexander Cooper
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |