1243418 - Fix swapped boolean usage and interpretation in GLUploadHelpers::UploadImageDataToTexture

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

The 'aOverwrite' argument, when true, is meant to indicate that you are uploading to a texture that already has data in it. Both the callers and implementation seem to have swapped that meaning.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Fix up incorrect 'aOverwrite' usage and impl in GLUploadHelpers

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8712720 [details] [diff] [review]
Fix up incorrect 'aOverwrite' usage and impl in GLUploadHelpers

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

The code is correct, but we should bother to clarify this, since it obviously wasn't clear enough before.

::: gfx/gl/GLUploadHelpers.cpp
@@ +432,4 @@
>                           const nsIntRegion& aDstRegion,
>                           GLuint& aTexture,
>                           size_t* aOutUploadSize,
>                           bool aOverwrite,

Why don't we just call this aNeedsInit? Would that be clearer?

::: gfx/gl/TextureImageEGL.cpp
@@ +213,5 @@
>                               aSurf,
>                               region,
>                               mTexture,
>                               &uploadSize,
> +                             mTextureState != Created,

Pull this out into a named local, which should help with clarity here.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3ee9f8005b3

Comment 4

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcbcaec45fc1

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/866edb59ba09

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/866edb59ba09

Comment 7

[Jonathan Kew [:jfkthame]]

This seems to have caused a regression related to hidpi support on OS X; filed bug 1247272.