Closed Bug 899811 Opened 6 years ago Closed 6 years ago

gfx/gl/GLContext.h consolidation

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: guillaume.abadie, Assigned: guillaume.abadie)

References

Details

Attachments

(2 files)

No description provided.
Attached patch patch revision 1Splinter Review
Attachment #783766 - Flags: review?(jgilbert)
Attachment #783766 - Flags: review?(bjacob)
Comment on attachment 783766 [details] [diff] [review]
patch revision 1

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

OK; please proofread the english comments a bit more, and please wait for jgilbert's review too.
Attachment #783766 - Flags: review?(bjacob) → review+
Comment on attachment 783766 [details] [diff] [review]
patch revision 1

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

I'm going to be doing a run through the comments to clean them up anyways, so don't worry too much about them for now.

::: gfx/gl/GLContext.h
@@ +354,5 @@
> +// -----------------------------------------------------------------------------
> +// XXX_* Extension group queries
> +/*
> + * This mecahnism introduce a new way to check if an extension is supported,
> + * regardless it is an ARB, EXT, OES, NV, AMD, APPLE, ANGLE ...

"mechanism introduces"
"regardless if it is an"
Maybe "ARB, EXT, OES, etc."
Attachment #783766 - Flags: review?(jgilbert) → review+
Attached patch landed patchSplinter Review
Here is the final landed patch.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a48c315c03c
Target Milestone: --- → mozilla25
The hg comments at least should contain the reason for this change and what was all done.  The diff is almost useless because of the stuff that may have moved, and it's unclear what was changed in the process.  Somebody else looking at this three months from now will have no idea as to the motivation for the change, nor what was actually done at the high level, what was accomplished, etc.  The comments in the code itself help, but it isn't enough.
Flags: needinfo?(gabadie)
Oups sorry,

This patch just move code in GLContext.h. The purpose of this patch was to restructure this huge class to be more user friendly. All very important information about the context that we would use very often are now at the top, while non standart entry points are nows at the bottom. And all feature are now shown with explicit separators between. For example, Error handling stuff is at the same place in the class, strongly separated from debug stuff.

It also adds const qualifiers on few functions that can have one. Also, this patch open the way for our extension group queries mechanism coming up, to explicitly say in comments that a entry points group are usable only if IsExtensionSupported(XXX_my_extension_group) is true.
Flags: needinfo?(gabadie)
https://hg.mozilla.org/mozilla-central/rev/9a48c315c03c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 900702
Blocks: 900964
You need to log in before you can comment on or make changes to this bug.