[blink][xr] Separate camera texture processing from OnFrameEnd() [chromium/src : main]
Mingjing Zhang (Gerrit)
Mingjing Zhang added 1 comment![]( "Open in Gerrit")
base_layer_->SubmitLayer();Note: this separation is more involved than the `OnFrameStart()` case. In the old `OnFrameEnd()`, the camera texture related code happens before `SubmitLayer()`, so when I isolated the camera texture code, I need to also isolate the `SubmitLayer()` code into a separate function to ensure the code execution order.
Related details
- Vasiliy Telezhnikov
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. |
Vasiliy Telezhnikov (Gerrit)
Vasiliy Telezhnikov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Vasiliy Telezhnikov
| Code-Review | +1 |
3 comments
base_layer_->SubmitLayer();Note: this separation is more involved than the `OnFrameStart()` case. In the old `OnFrameEnd()`, the camera texture related code happens before `SubmitLayer()`, so when I isolated the camera texture code, I need to also isolate the `SubmitLayer()` code into a separate function to ensure the code execution order.
I think ordering doesn't really matter, so we could call `OnFrameEndForCamera` before `OnFrameEnd`.
Maybe we should land this CL (% another comment) as is and then in a separate CL reorder `OnFrameEndWithoutSubmit` and `OnFrameEndForCamera` (with a kill-switch if you prefer).
After that we can merge `OnFrameEndWithoutSubmit()` and `SubmitLayer()` (eventually).
But this won't block us moving `OnFrameStartForCamera` and `OnFrameEndForCamera` to XRRenderState with relevant variables.
if (camera_image_texture_scoped_access_) {
gpu::SharedImageTexture::ScopedAccess::EndAccess(
std::move(camera_image_texture_scoped_access_));
camera_image_shared_image_texture_.reset();
}nit: I think we should move this too. Ordering wise it wil stay the same, `OnFrameEndWithoutSubmit` will call only the part above if session ended and return early and `OnFrameEndForCamera` would call camera part and return early.
Related details
- Mingjing Zhang
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. |
Mingjing Zhang (Gerrit)
Mingjing Zhang added 2 comments![]( "Open in Gerrit")
Vasiliy TelezhnikovNote: this separation is more involved than the `OnFrameStart()` case. In the old `OnFrameEnd()`, the camera texture related code happens before `SubmitLayer()`, so when I isolated the camera texture code, I need to also isolate the `SubmitLayer()` code into a separate function to ensure the code execution order.
I think ordering doesn't really matter, so we could call `OnFrameEndForCamera` before `OnFrameEnd`.
Maybe we should land this CL (% another comment) as is and then in a separate CL reorder `OnFrameEndWithoutSubmit` and `OnFrameEndForCamera` (with a kill-switch if you prefer).
After that we can merge `OnFrameEndWithoutSubmit()` and `SubmitLayer()` (eventually).
But this won't block us moving `OnFrameStartForCamera` and `OnFrameEndForCamera` to XRRenderState with relevant variables.
Acknowledged
if (camera_image_texture_scoped_access_) {
gpu::SharedImageTexture::ScopedAccess::EndAccess(
std::move(camera_image_texture_scoped_access_));
camera_image_shared_image_texture_.reset();
}nit: I think we should move this too. Ordering wise it wil stay the same, `OnFrameEndWithoutSubmit` will call only the part above if session ended and return early and `OnFrameEndForCamera` would call camera part and return early.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Cooper (Gerrit)
Alexander Cooper added 3 comments![]( "Open in Gerrit")
Now that camera textures are only associated with the base layer, we
no longer need the camera texture processing logic in OnFrameEnd() for
non-base layer cases. In this CL, we extract the camera textureI wonder if it wouldn't just be cleaner to pass a value when creating the base layer of if it needs to be concerned about the camera image or not, since not every session has the camera image, and I can imagine that in the future we may want to special case how the camera image is handled when we support e.g. not using the base layers but still providing the camera image.
base_layer_->OnFrameEndWithoutSubmit();The creation of this method doesn't seem documented, and I'm a bit confused why it is needed?
if (session()->ended()) {Add comment about session end here as well.
Related details
- Mingjing Zhang
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. |