Merge LayerOGL CleanupResources and Destroy

ASSIGNED
Assigned to

Status

()

ASSIGNED
6 years ago
6 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
The LayerOGL tree has two similar recursive methods, Destroy and
CleanupResources, each with implementations on each of the layer classes.
These methods are similar enough that we should be able to merge these two forms into a single method on each class.

Currently the CleanupResources methods are not really used, from Gecko at
least, and so the simple approach is to remove them.  I'll attach patches here
to do that.  LayerManagerOGL::CleanupResources was already removed in bug
630346 because it was not called.

CleanupResources methods were added in bug 715822, but even in Firefox 12
there were no callers of the LayerManagerOGL method.  Is there an external
caller somewhere?

The CleanupResources methods seem to have bugs if they are ever called twice
because they try to delete textures of the same name again.

If there is reason to keep the CleanupResources methods, I can look at instead
removing the LayerOGL::Destroy methods and making do with CleanupResources.
(Assignee)

Comment 1

6 years ago
Created attachment 663942 [details] [diff] [review]
part 1: remove unused LayerOGL::CleanupResources methods
Assignee: nobody → karlt
Status: NEW → ASSIGNED
Attachment #663942 - Flags: review?(matt.woodrow)
(Assignee)

Updated

6 years ago
Blocks: 715822
(Assignee)

Comment 2

6 years ago
Created attachment 663944 [details] [diff] [review]
part 2: merge remaining CleanupResources layer methods into Destroy methods
Attachment #663944 - Flags: review?(matt.woodrow)
(Assignee)

Comment 3

6 years ago
Looks like Bug 806029 will add a CleanupResources caller intended for releasing resources on shadow layers (though it would also be called on regular layers if ClearCachedResources were called on their layer manager).
Matt, can you review this?
I can review this as well. I took a look and these patches are a nice clean up but they have rotted. For example we since have usage to CleanupResources here:
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/LayerManagerOGL.cpp#876

I'd be happy to r+ this with the rot fixed.
(Assignee)

Updated

6 years ago
Attachment #663942 - Flags: review?(matt.woodrow)
(Assignee)

Updated

6 years ago
Attachment #663944 - Flags: review?(matt.woodrow)
You need to log in before you can comment on or make changes to this bug.