912816 - Always create a new texture when we update with CompositorOGL

Status: NEW

(Core :: Graphics: Layers, defect)

Description

[Nick Cameron [:nrc]]

With OpenGL textures we currently avoid creating a new texture when we update (http://dxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/TextureHostOGL.cpp#l447). On the d3d backends we always create a new texture. Should we do this on OpenGL too? Do we ever block if a texture is in use when we try to update?

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Can you please provide more background on why you think that it could be useful to always create a new texture?

A priori, an obvious concern is that creating new textures all the time may be slow, and/or may be a more difficult exercise for the driver's memory allocation mechanisms to get right.

Comment 2

[Kelsey Gilbert [:jgilbert]]

(In reply to Nick Cameron [:nrc] from comment #0)
> With OpenGL textures we currently avoid creating a new texture when we
> update
> (http://dxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/
> TextureHostOGL.cpp#l447). On the d3d backends we always create a new
> texture. Should we do this on OpenGL too? Do we ever block if a texture is
> in use when we try to update?

I believe the consensus is that creating a new texture object can be slower than updating an existing one. I saw some more information somewhere recently, so I'll try to dig that up.

That said, in OpenGL, TexImage2D respecifies the texture, so the texture data is (probably) recreated anyways, unless the sizes matched. (In which case we should just do TexSubImage)

Comment 3

[Andreas Gal :gal]

Do we have proof (profile) for comment 2?

Comment 4

[Kelsey Gilbert [:jgilbert]]

(In reply to Andreas Gal :gal from comment #3)
> Do we have proof (profile) for comment 2?

I don't have anything, no. Creating a new texture in OpenGL should never be faster than using an old one, though. (It might not be slower, but shouldn't ever be faster)

TexImage2D effectively reallocs the object anyways. If the object is in-use, it won't block on the read lock in the driver server thread, it should just spin off a new texture object, and let the old info get collected via refcounting. (this is what the spec points to, at least)

I don't think this is worth investigating from a perf perspective.

Particularly if we're just updating the data in a texture, and leaving the size unchanged, we should absolutely be using TexSubImage2D, which *should* give us a perf advantage over TexImage2D.

(I can't find the resource I was looking for from comment 2, though)

Comment 5

[Andreas Gal :gal]

TexSubImage2D has known performance problems around bad drivers but also texture locking. Without evidence that this saves us a lot of time I would recommend using a new texture and we can optimize this the moment we see it be slow. wdyt?

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Andreas Gal :gal from comment #5)
> TexSubImage2D has known performance problems around bad drivers but also
> texture locking. Without evidence that this saves us a lot of time I would
> recommend using a new texture and we can optimize this the moment we see it
> be slow. wdyt?

While I don't have any data, it is widely believed that creating new objects should be kept to a minimum. I would rather not go against conventional wisdom without data.

It still hasn't been explained *why* this change is desired. As far as I can tell, there's no real reason for us to want to use a new texture over using an old texture save for hypothetical performance reasons, reasons which go against standard practice.

I think it's much more reasonable to keep the status quo (and standard practice) for now, until we get data showing affirmatively that the standard practice is wrong in this case.

Comment 7

[Andreas Gal :gal]

Where does this standard practice come from? Any references? I am not doubting what you are saying, I am simply curious and I would like to make sure we make good decisions. Sources help with that. We have clear evidence that on mobile reusing textures can cause all sorts of driver locking problems, documented in several bugs.

Comment 8

[Matt Woodrow (:mattwoodrow)]

Can we clarify what we mean exactly by creating new textures in this case.

Are we talking

glDeleteTextures, glGenTextures, glTexImage2D
vs
glTexImage2D


or instead

glTexImage2D
vs
glTexSubImage2D(0,0,width, height)


If it's the former, then I can't see any reason why we'd want to create new textures. The latter is more interesting, since we've had problems in the past with glTexSubImage2D being slow on mobile (I assume because it requires a pipeline flush to ensure that nothing is depending on the current texture data, and probably just an awful lack of optimization).