Fix swapped boolean usage and interpretation in GLUploadHelpers::UploadImageDataToTexture

RESOLVED FIXED in Firefox 47

Status

()

Core
Graphics
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: snorp, Assigned: snorp)

Tracking

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

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.
Created attachment 8712720 [details] [diff] [review]
Fix up incorrect 'aOverwrite' usage and impl in GLUploadHelpers
Attachment #8712720 - Flags: review?(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.
Attachment #8712720 - Flags: review?(jgilbert) → review+
Assignee: nobody → snorp

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/866edb59ba09
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Depends on: 1247272
This seems to have caused a regression related to hidpi support on OS X; filed bug 1247272.
Duplicate of this bug: 1256318
Depends on: 1256576
You need to log in before you can comment on or make changes to this bug.