Closed Bug 966999 Opened 6 years ago Closed 6 years ago

Remove all the deprecated texture code that is not used by Thebes layers

Categories

(Core :: Graphics: Layers, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: nical, Assigned: nical)

References

Details

Attachments

(2 files, 1 obsolete file)

As soon as Image and Canvas layers are not using deprecated code on all platforms we can remove a whole bunch of deprecated code.
Attached patch Remove a lot of deprecated code (obsolete) — Splinter Review
Attachment #8369433 - Flags: review?(vladimir)
Comment on attachment 8369433 [details] [diff] [review]
Remove a lot of deprecated code

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

I am excite! A few minor comments, msotly style.  They're all with TYCbCrImage, so maybe just replace all those with a MOZ_ASSERT(foo != TYCbCrImage) at the start of the functions?

::: gfx/layers/basic/BasicCompositor.cpp
@@ +210,5 @@
>                               uint32_t aTextureFlags)
>  {
>    RefPtr<DeprecatedTextureHost> result = nullptr;
>    if (aDescriptorType == SurfaceDescriptor::TYCbCrImage) {
> +    MOZ_CRASH("Should be using non-deprecated textures");

any reason to have this here at all, instead of just getting rid of the if block entirely and leaving the else block with the MOZ_ASSERT?  With the message in the MOZ_ASSERT changed to something more appropriate.

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +418,5 @@
>                                   uint32_t aTextureFlags)
>  {
>    RefPtr<DeprecatedTextureHost> result;
>    if (aDescriptorType == SurfaceDescriptor::TYCbCrImage) {
> +    MOZ_CRASH("Should be using non-deprecated textures");

Same comment as before.. I kind of don't like having the first if block lead to a MOZ_CRASH :)

::: gfx/layers/d3d9/TextureD3D9.cpp
@@ +25,5 @@
>                        uint32_t aTextureFlags)
>  {
>    RefPtr<DeprecatedTextureHost> result;
>    if (aDescriptorType == SurfaceDescriptor::TYCbCrImage) {
> +    MOZ_CRASH("Should be using non-deprecated textures");

Ditto. Though this one is even more complicated to untangle.  Maybe just MOZ_ASSERT(aDescriptorType != TYCbCrImage) ?
Attachment #8369433 - Flags: review?(vladimir) → review+
Fixed that by removing YCbCrImage and RGBImage entirely form the surface descriptor types.
Attachment #8369433 - Attachment is obsolete: true
Attachment #8370002 - Flags: review?(vladimir)
Duplicate of this bug: 847914
I uploaded and got this patch reviewed by vlad on the wrong bug (Bug 965804 which is closed) So I guess it's better that I land it with a bug that is still open. Sorry for the confusion.
Comment on attachment 8370002 [details] [diff] [review]
Remove a lot of deprecated code

Gah sorry, this got lost in my tracking.
Attachment #8370002 - Flags: review?(vladimir) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.