Closed Bug 1400407 Opened 2 years ago Closed 2 years ago

Cleanup WebVR Dead Code

Categories

(Core :: WebVR, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: kip, Assigned: kip)

References

Details

Attachments

(3 files)

Once Bug 1381085 lands, WebVR won't be dependent on some functions that can be refactored out and deleted.
This cleanup can wait until FF58+
Note that Bug 1381085 must land first
Depends on: 1381085
The dependency patch from Bug 1381085 has been moved to Bug 1400457.

Please do not land this until after Bug 1400457 lands.
Depends on: 1400457
No longer depends on: 1381085
Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190738

::: gfx/gl/SharedSurface.cpp:326
(Diff revision 3)
>      while (!mRecycleFreePool.empty()) {
>          RefPtr<layers::SharedSurfaceTextureClient> cur = mRecycleFreePool.front();
>          mRecycleFreePool.pop();
>  
>          if (cur->Surf()->mSize == size){
> -            // In the general case, textureClients transit textures through
> +            if (aLayersChannel && aLayersChannel == cur->GetAllocator()) {

@jgilbert: VRManagerChild is no longer a TextureForwarder, so I am removing the special case check for VRMAnagerChild here.
Attachment #8908849 - Flags: review?(jgilbert)
Attachment #8908849 - Flags: review?(dmu)
@jgilbert: For the part1 patch, could you take a look at the change to gfx/gl/SharedSurface.cpp?  See Comment 15 for some context.
@daoshengmu: Could you please review the rest of the patch?
Flags: needinfo?(jgilbert)
Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

https://reviewboard.mozilla.org/r/180466/#review190742

::: commit-message-14b36:5
(Diff revision 4)
> +Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code
> +- Refactored out Canvas layer mirror specialization, no longer needed
> +  as we are no longer implementing a TextureForwarder to submit VR frames.
> +
> +MozReview-Commit-ID: ArZO6M9kNLg

I am assuming that the "mirror" functionality won't be needed in the future for things other than WebVR.  As WebVR is no longer dependent on it, I suspect you will be glad to see this [now dead code] cleaned up.  Please advise if otherwise.
Attachment #8908850 - Flags: review?(jgilbert)
Attachment #8908851 - Flags: review?(jgilbert)
Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

https://reviewboard.mozilla.org/r/180468/#review190744

::: commit-message-e070f:1
(Diff revision 4)
> +Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

This is again the same patch from Bug 1382104 - Remove IsMirror concept in favor of checking forwarders.

In this case; however, we don't have to worry about the "checking forwarders" part, as the mirroring code will not be hit at all any more.
Comment on attachment 8908850 [details]
Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code

https://reviewboard.mozilla.org/r/180466/#review190838
Attachment #8908850 - Flags: review?(jgilbert) → review+
Comment on attachment 8908851 [details]
Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR

https://reviewboard.mozilla.org/r/180468/#review190840
Attachment #8908851 - Flags: review?(jgilbert) → review+
Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190844
Attachment #8908849 - Flags: review?(jgilbert) → review+
(In reply to :kip (Kearwood Gilbert) from comment #15)
> Comment on attachment 8908849 [details]
> Bug 1400407 - Part 1: Cleanup WebVR dead code
> 
> https://reviewboard.mozilla.org/r/180464/#review190738
> 
> ::: gfx/gl/SharedSurface.cpp:326
> (Diff revision 3)
> >      while (!mRecycleFreePool.empty()) {
> >          RefPtr<layers::SharedSurfaceTextureClient> cur = mRecycleFreePool.front();
> >          mRecycleFreePool.pop();
> >  
> >          if (cur->Surf()->mSize == size){
> > -            // In the general case, textureClients transit textures through
> > +            if (aLayersChannel && aLayersChannel == cur->GetAllocator()) {
> 
> @jgilbert: VRManagerChild is no longer a TextureForwarder, so I am removing
> the special case check for VRMAnagerChild here.

This change is fine.
Flags: needinfo?(jgilbert)
Comment on attachment 8908849 [details]
Bug 1400407 - Part 1: Cleanup WebVR dead code

https://reviewboard.mozilla.org/r/180464/#review190888

LGTM.
Attachment #8908849 - Flags: review?(dmu) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 70bc869c7e58 -d b0c2046983c6: rebasing 423904:70bc869c7e58 "Bug 1400407 - Part 1: Cleanup WebVR dead code r=daoshengmu,jgilbert"
merging gfx/vr/ipc/VRLayerParent.h
rebasing 423905:7fb46e4d60a5 "Bug 1400407 - Part 2: Cleanup DOM Canvas mirroring dead code r=jgilbert"
merging dom/canvas/CanvasRenderingContext2D.cpp
merging dom/canvas/WebGLContext.h
rebasing 423906:dabd62b62a07 "Bug 1400407 - Part 3: Remove IsMirror concept, as it is no longer used by WebVR r=jgilbert" (tip)
merging dom/canvas/WebGLContext.cpp
merging dom/canvas/WebGLContext.h
warning: conflicts while merging dom/canvas/WebGLContext.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by kgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6daba05eb240
Part 1: Cleanup WebVR dead code,r=daoshengmu,r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/53c7ef6743d9
Part 2: Cleanup DOM Canvas mirroring dead code,r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/83203c2de90a
Part 3: Remove IsMirror concept, as it is no longer used by WebVR,r=jgilbert
You need to log in before you can comment on or make changes to this bug.