Closed Bug 919022 Opened 6 years ago Closed 6 years ago

CompositableClient.h: 'virtual mozilla::TemporaryRef<mozilla::layers::BufferTextureClient> mozilla::layers::CompositableClient::CreateBufferTextureClient(mozilla::gfx::SurfaceFormat, mozilla::layers::TextureFlags)' was hidden

Categories

(Core :: Graphics: Layers, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: dholbert, Assigned: nical)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I noticed this build warning spammed twice during my mozilla-inbound build today (once for each .cpp file that #includes CanvasClient.h, directly or indirectly):

{
layers/CompositableClient.h: 'virtual mozilla::TemporaryRef<mozilla::layers::BufferTextureClient> mozilla::layers::CompositableClient::CreateBufferTextureClient(mozilla::gfx::SurfaceFormat, mozilla::layers::TextureFlags)' was hidden

    CreateBufferTextureClient(gfx::SurfaceFormat aFormat, TextureFlags aFlags);
    ^

layers/CanvasClient.h:86:3: warning:   by 'virtual mozilla::TemporaryRef<mozilla::layers::BufferTextureClient> mozilla::layers::CanvasClient2D::CreateBufferTextureClient(mozilla::gfx::SurfaceFormat)' [-Woverloaded-virtual]

    CreateBufferTextureClient(gfx::SurfaceFormat aFormat) MOZ_OVERRIDE;
    ^
}

I believe the issue is that:

 - The parent class has two functions named CreateBufferTextureClient() with different args.
 - The child class only overrides *one* of them.
 - This "hides" the other one, from the perspective of the child class (i.e. any hypothetical invocation of CreateBufferTextureClient() on the child class will be unable to invoke the other inherited method, though it'd work if you cast the instance to have the parent class's type.)

...and the compiler thinks this is something it should warn about, messily.

I don't think this is a real code bug -- it looks like we intentionally only overrode one method, since the overriding implementation explicitly invokes the other method, using the parent class's name. So that's good. It'd be nice to convince the compiler that we're OK here, though, so that it quiets down.
Meant to say -- the parent's second copy of this method (the 2-arg version of "CompositableClient::CreateBufferTextureClient"), as well as the the child override for the 1-arg version, were both added in this commit for bug 900133:
  http://hg.mozilla.org/mozilla-central/rev/64ab855db547
so I'm pretty sure that's what introduced this warning.
Assignee: nobody → nical.bugzilla
Depends on: 916118
Attachment #810632 - Flags: review?(ncameron)
Comment on attachment 810632 [details] [diff] [review]
Do not overload CreateBufferTextureClient

Review of attachment 810632 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/client/ImageClient.cpp
@@ -284,5 @@
> -TemporaryRef<BufferTextureClient>
> -ImageClientSingle::CreateBufferTextureClient(gfx::SurfaceFormat aFormat)
> -{
> -  return CompositableClient::CreateBufferTextureClient(aFormat,
> -    mTextureFlags | TEXTURE_FLAGS_DEFAULT);

You seem to be changing the semantics here, a call to CreateBufferTextureClient with one arg will no longer get mTextureFlags. Is that OK?
[adding needinfo=nical, per comment 3]
Flags: needinfo?(nical.bugzilla)
(In reply to Nick Cameron [:nrc] from comment #3)
> Comment on attachment 810632 [details] [diff] [review]
> Do not overload CreateBufferTextureClient
> 
> Review of attachment 810632 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/client/ImageClient.cpp
> @@ -284,5 @@
> > -TemporaryRef<BufferTextureClient>
> > -ImageClientSingle::CreateBufferTextureClient(gfx::SurfaceFormat aFormat)
> > -{
> > -  return CompositableClient::CreateBufferTextureClient(aFormat,
> > -    mTextureFlags | TEXTURE_FLAGS_DEFAULT);
> 
> You seem to be changing the semantics here, a call to
> CreateBufferTextureClient with one arg will no longer get mTextureFlags. Is
> that OK?

It does, the one arg version gets mTextureFlags | aFlags
see line 281:

return CompositableClient::CreateBufferTextureClient(aFormat, mTextureFlags | aFlags);
Flags: needinfo?(nical.bugzilla)
Attachment #810632 - Flags: review?(ncameron) → review+
https://hg.mozilla.org/mozilla-central/rev/03dfed609009
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.