Support debug trace printouts in EGL

RESOLVED FIXED in mozilla13

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Joe Drew (not getting mail), Assigned: Joe Drew (not getting mail))

Tracking

unspecified
mozilla13
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 594331 [details] [diff] [review]
debug output egl

There is currently no MOZ_GL_DEBUG_VERBOSE support from EGL functions, which can be a little annoying. This patch implements that (and probably has some other cruft in it that needs to be fixed before committing).

(Specifically, all the #ifdef DEBUG -> #if 1 changes will need to be reverted)
Attachment #594331 - Flags: review?(bjacob)
Aside: have you given apitrace a spin?
(Assignee)

Comment 2

5 years ago
Benoit got the apitrace patches to apply and compile again, but says it crashed when he tried to use it. Neither of us have tried more than that.
Comment on attachment 594331 [details] [diff] [review]
debug output egl

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

::: gfx/gl/GLContext.cpp
@@ +58,5 @@
>  
>  namespace mozilla {
>  namespace gl {
>  
> +#if 1

Did you really intend to get review for this patch? The above makes it look more like a useful local patch, not for landing as-is.
Attachment #594331 - Flags: review?(bjacob)
(Assignee)

Updated

5 years ago
Assignee: nobody → joe
(Assignee)

Comment 4

5 years ago
Created attachment 594812 [details] [diff] [review]
add EGL functions output ifdef DEBUG

That's why I said "(Specifically, all the #ifdef DEBUG -> #if 1 changes will need to be reverted)" :)

Anyways, here is a cleaned-up version.
Attachment #594331 - Attachment is obsolete: true
Attachment #594812 - Flags: review?(bjacob)
Woops, sorry about that.
Comment on attachment 594812 [details] [diff] [review]
add EGL functions output ifdef DEBUG

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

::: gfx/gl/GLContextProviderEGL.cpp
@@ +672,5 @@
>              return false;
>          }
>  
>  #define SYMBOL(name) \
> +    { (PRFuncPtr*) &mSymbols.f##name, { "egl" #name, NULL } }

Hah, that's clever. I wonder if it can be a little bit dangerous to use such a common name as SYMBOL for that, but that's a cpp, not a header, after all.

Are you going to generalize this trick to all our *GL symbols? I would, for once, support this use of macros!
Attachment #594812 - Flags: review?(bjacob) → review+
(Assignee)

Comment 7

5 years ago
Note that I didn't create SYMBOL, I just changed its implementation.

I don't currently have plans to generalize this trick, as I'm focused on EGL, but I have no particular issues with doing it some time in the mysterious future.

Comment 8

5 years ago
Try run for cd3085bdfb4c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=cd3085bdfb4c
Results (out of 30 total builds):
    success: 29
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdrew@mozilla.com-cd3085bdfb4c

Comment 9

5 years ago
Try run for 74b43278cbcd is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=74b43278cbcd
Results (out of 28 total builds):
    success: 25
    warnings: 1
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdrew@mozilla.com-74b43278cbcd
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4799638be1d
https://hg.mozilla.org/mozilla-central/rev/c4799638be1d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.