Add OpenXR mesh extension support and API wrappers [chromium/src : main]
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
- Adam Ren (xWF)
- Alexander Cooper
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 5 comments![]( "Open in Gerrit")
return mesh_manager_.get();NIT: Maybe check feature enabled state?
#if BUILDFLAG(IS_ANDROID)Shouldn't need to be wrapped in `IS_ANDROID` (and surprised it didn't give other errors). Realistically they'll only be present on Android, but when these methods make it into the OpenXR header, they won't need to be restricted anymore.
feature == device::mojom::XRSessionFeature::MESH_DETECTION;Should be reverted, at least for now.
// Concrete implementations (like for Android or HoloLens) will inherit from
// this.Remove.
Related details
- Adam Ren (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. |
Adam Ren (xWF) (Gerrit)
Adam Ren (xWF) added 4 comments![]( "Open in Gerrit")
NIT: Maybe check feature enabled state?
In this CL, it's just like a placeholder. It has been modified to check the enabled state in the fourth CL.
Shouldn't need to be wrapped in `IS_ANDROID` (and surprised it didn't give other errors). Realistically they'll only be present on Android, but when these methods make it into the OpenXR header, they won't need to be restricted anymore.
Done
feature == device::mojom::XRSessionFeature::MESH_DETECTION;Should be reverted, at least for now.
Done
// Concrete implementations (like for Android or HoloLens) will inherit from
// this.Adam Ren (xWF)Remove.
Done
Related details
- Alexander Cooper
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 2 comments![]( "Open in Gerrit")
return mesh_manager_.get();Adam Ren (xWF)NIT: Maybe check feature enabled state?
In this CL, it's just like a placeholder. It has been modified to check the enabled state in the fourth CL.
If just a placeholder, maybe it should be nullptr? While the goal is obviously for all of these to be merged, it's reasonable to not necessarily merge these all at once, e.g. merging the earlier ones as they are ready, and further each *is* a distinct CL, so should be left in a good "intermediate" state. A blind return of the type without a feature check *could* turn into a bug if e.g. this was merged now and then the later CL was massively re-worked.
#include "third_party/openxr/dev/xr_android.h"NIT: Needs to be sorted as it was originally as well.
Related details
- Adam Ren (xWF)
- Brandon Jones
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. |
Adam Ren (xWF) (Gerrit)
Adam Ren (xWF) added 2 comments![]( "Open in Gerrit")
return mesh_manager_.get();Adam Ren (xWF)NIT: Maybe check feature enabled state?
Alexander CooperIn this CL, it's just like a placeholder. It has been modified to check the enabled state in the fourth CL.
If just a placeholder, maybe it should be nullptr? While the goal is obviously for all of these to be merged, it's reasonable to not necessarily merge these all at once, e.g. merging the earlier ones as they are ready, and further each *is* a distinct CL, so should be left in a good "intermediate" state. A blind return of the type without a feature check *could* turn into a bug if e.g. this was merged now and then the later CL was massively re-worked.
Thanks. Updated to return nullptr as a placeholder so this CL is safe to merge on its own.
NIT: Needs to be sorted as it was originally as well.
- Alexander Cooper
- Brandon Jones
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 voted Code-Review+1![]( "Open in Gerrit")
- Adam Ren (xWF)
- Brandon Jones
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 1 comment![]( "Open in Gerrit")
There should be a bug associated with all of these CLs
Related details
- Adam Ren (xWF)
- Brandon Jones
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. |
Adam Ren (xWF) (Gerrit)
Adam Ren (xWF) added 1 comment![]( "Open in Gerrit")
There should be a bug associated with all of these CLs
- Brandon Jones
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |