899811 - gfx/gl/GLContext.h consolidation

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[Guillaume Abadie]

Attachment: patch revision 1

Comment 2

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

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.

Comment 3

[Guillaume Abadie]

https://tbpl.mozilla.org/?tree=Try&rev=d04e2b368ccf

Comment 4

[Kelsey Gilbert [:jgilbert]]

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."

Comment 5

[Guillaume Abadie]

Attachment: landed patch

Here is the final landed patch.

Comment 6

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a48c315c03c

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

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.

Comment 8

[Guillaume Abadie]

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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a48c315c03c