Summary: | Use frameData instead of scheduleOnNextFrame calls in WebFakeXRDevice | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Imanol Fernandez <ifernandez> | ||||||
Component: | WebXR | Assignee: | Imanol Fernandez <ifernandez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 208988 | ||||||||
Attachments: |
|
Description
Imanol Fernandez
2021-03-22 09:06:00 PDT
Created attachment 423896 [details]
Patch
Comment on attachment 423896 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423896&action=review > Source/WebCore/testing/WebFakeXRDevice.cpp:63 > +void SimulatedXRDevice::setViews(Vector<Ref<FakeXRView>>&& views) Does it need to be a &&? > Source/WebCore/testing/WebFakeXRDevice.cpp:84 > +void SimulatedXRDevice::setViewerOrigin(Optional<FrameData::Pose>&& origin) const Optional<>& > Source/WebCore/testing/WebFakeXRDevice.cpp:89 > + m_frameData.isTrackingValid = true; return here. > Source/WebCore/testing/WebFakeXRDevice.cpp:138 > + FrameData data = m_frameData; Should we do FrameData data = WTFMove(m_frameData)? > Source/WebCore/testing/WebFakeXRDevice.cpp:202 > + deviceViews.append(view.releaseReturnValue()); I would tend to remove the other setViews and create the FrameData::View object here. > Source/WebCore/testing/WebXRTest.cpp:57 > + simulatedDevice.setViews(WTFMove(views)); Cannot we directly call setViews(const Vector<FakeXRViewInit>&) Comment on attachment 423896 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423896&action=review >> Source/WebCore/testing/WebFakeXRDevice.cpp:63 >> +void SimulatedXRDevice::setViews(Vector<Ref<FakeXRView>>&& views) > > Does it need to be a &&? I changed it to Vector<FrameData::View>&& >> Source/WebCore/testing/WebFakeXRDevice.cpp:84 >> +void SimulatedXRDevice::setViewerOrigin(Optional<FrameData::Pose>&& origin) > > const Optional<>& done >> Source/WebCore/testing/WebFakeXRDevice.cpp:89 >> + m_frameData.isTrackingValid = true; > > return here. done >> Source/WebCore/testing/WebFakeXRDevice.cpp:138 >> + FrameData data = m_frameData; > > Should we do FrameData data = WTFMove(m_frameData)? No, because we want m_fameData to keep the previous values. >> Source/WebCore/testing/WebFakeXRDevice.cpp:202 >> + deviceViews.append(view.releaseReturnValue()); > > I would tend to remove the other setViews and create the FrameData::View object here. Done >> Source/WebCore/testing/WebXRTest.cpp:57 >> + simulatedDevice.setViews(WTFMove(views)); > > Cannot we directly call setViews(const Vector<FakeXRViewInit>&) yes, done. Created attachment 425381 [details]
Patch for landing
Committed r275603: <https://commits.webkit.org/r275603> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425381 [details]. |