Closed Bug 997374 Opened 10 years ago Closed 10 years ago

Check FB status in GLScreenBuffer

Categories

(Core :: Graphics: CanvasWebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jgilbert, Assigned: jgilbert)

References

Details

Attachments

(1 file)

Right now, we assert that our FBs are complete. They might not be, so we should check, and fail gracefully if they fail.
Attached patch check-fbSplinter Review
Attachment #8407770 - Flags: review?(bjacob)
I started hitting these asserts in bug 996266, with this try run on WinXP:
https://tbpl.mozilla.org/?tree=Try&rev=7196140c4056
Blocks: 996266
Comment on attachment 8407770 [details] [diff] [review]
check-fb

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

::: gfx/gl/GLScreenBuffer.cpp
@@ +405,3 @@
>          ReadBuffer* read = CreateRead(surf);
> +        if (!read)
> +            ok = false;

Rather than having a single 'ok' variable that gets mutated, changing its meaning, I would rather have separate immutable bools. It would make the flow of logic easier for me to follow, and the names of these variables would have to be more specific, self-documenting them. Does that make sense?

bool drawOk = CreateDraw(size, &draw);
ReadBuffer* read = CreateRead(surf);
bool readOk = read != nullptr;
bool ok = drawOk && readOk;

::: gfx/gl/GLScreenBuffer.h
@@ +51,5 @@
> +    static bool Create(GLContext* const gl,
> +                       const SurfaceCaps& caps,
> +                       const GLFormats& formats,
> +                       const gfx::IntSize& size,
> +                       DrawBuffer** out_buffer);

A two-star michelin pointer. Delicious.
Attachment #8407770 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/mozilla-central/rev/c8871dd41a10
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.