Last Comment Bug 758844 - Quiet graphics code during startup
: Quiet graphics code during startup
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 All
: -- normal (vote)
: mozilla17
Assigned To: John Drinkwater (:beta)
:
Mentors:
Depends on: 762265
Blocks: fx-noise
  Show dependency treegraph
 
Reported: 2012-05-25 18:10 PDT by Jesse Ruderman
Modified: 2012-07-18 05:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Disable debug output unless MOZ_GL_DEBUG is defined (1.05 KB, patch)
2012-06-01 08:24 PDT, John Drinkwater (:beta)
jacob.benoit.1: review-
Details | Diff | Review
Disable debug output unless MOZ_GL_DEBUG is defined (2.73 KB, patch)
2012-06-01 10:43 PDT, John Drinkwater (:beta)
jacob.benoit.1: review+
Details | Diff | Review
Disable debug output unless MOZ_GL_DEBUG is defined (2.71 KB, patch)
2012-06-01 12:22 PDT, John Drinkwater (:beta)
john: review+
Details | Diff | Review
Disable most WEBGL debug output unless env vars set. (7.83 KB, patch)
2012-06-06 10:55 PDT, John Drinkwater (:beta)
jgilbert: review+
Details | Diff | Review
Revert some changes from Bug 762265 (1.17 KB, patch)
2012-06-16 08:10 PDT, John Drinkwater (:beta)
no flags Details | Diff | Review
Quiet GFX spew except on the first run in DebugMode() (4.22 KB, patch)
2012-07-11 19:54 PDT, Jeff Gilbert [:jgilbert]
jacob.benoit.1: review+
Details | Diff | Review

Description Jesse Ruderman 2012-05-25 18:10:47 PDT
People who aren't debugging graphics code don't need this on every startup. It gets in the way of seeing interesting things like warnings.

OpenGL vendor ('ATI Technologies Inc.') recognized as: ATI
GL extensions: GL_ARB_color_buffer_float GL_ARB_depth_buffer_float GL_ARB_depth_clamp GL_ARB_depth_texture GL_ARB_draw_buffers GL_ARB_draw_elements_base_vertex GL_ARB_draw_instanced GL_ARB_fragment_program GL_ARB_fragment_program_shadow GL_ARB_fragment_shader GL_ARB_framebuffer_object GL_ARB_framebuffer_sRGB GL_ARB_half_float_pixel GL_ARB_half_float_vertex GL_ARB_imaging GL_ARB_instanced_arrays GL_ARB_multisample GL_ARB_multitexture GL_ARB_occlusion_query GL_ARB_pixel_buffer_object GL_ARB_point_parameters GL_ARB_point_sprite GL_ARB_provoking_vertex GL_ARB_seamless_cube_map GL_ARB_shader_objects GL_ARB_shader_texture_lod GL_ARB_shading_language_100 GL_ARB_shadow GL_ARB_shadow_ambient GL_ARB_sync GL_ARB_texture_border_clamp GL_ARB_texture_compression GL_ARB_texture_compression_rgtc GL_ARB_texture_cube_map GL_ARB_texture_env_add GL_ARB_texture_env_combine GL_ARB_texture_env_crossbar GL_ARB_texture_env_dot3 GL_ARB_texture_float GL_ARB_texture_mirrored_repeat GL_ARB_texture_non_power_of_two GL_ARB_texture_rectangle GL_ARB_texture_rg GL_ARB_transpose_matrix GL_ARB_vertex_array_bgra GL_ARB_vertex_blend GL_ARB_vertex_buffer_object GL_ARB_vertex_program GL_ARB_vertex_shader GL_ARB_window_pos GL_EXT_abgr GL_EXT_bgra GL_EXT_bindable_uniform GL_EXT_blend_color GL_EXT_blend_equation_separate GL_EXT_blend_func_separate GL_EXT_blend_minmax GL_EXT_blend_subtract GL_EXT_clip_volume_hint GL_EXT_draw_buffers2 GL_EXT_draw_range_elements GL_EXT_fog_coord GL_EXT_framebuffer_blit GL_EXT_framebuffer_multisample GL_EXT_framebuffer_object GL_EXT_framebuffer_sRGB GL_EXT_geometry_shader4 GL_EXT_gpu_program_parameters GL_EXT_gpu_shader4 GL_EXT_multi_draw_arrays GL_EXT_packed_depth_stencil GL_EXT_packed_float GL_EXT_provoking_vertex GL_EXT_rescale_normal GL_EXT_secondary_color GL_EXT_separate_specular_color GL_EXT_shadow_funcs GL_EXT_stencil_two_side GL_EXT_stencil_wrap GL_EXT_texture_array GL_EXT_texture_compression_dxt1 GL_EXT_texture_compression_s3tc GL_EXT_texture_env_add GL_EXT_texture_filter_anisotropic GL_EXT_texture_integer GL_EXT_texture_lod_bias GL_EXT_texture_mirror_clamp GL_EXT_texture_rectangle GL_EXT_texture_shared_exponent GL_EXT_texture_sRGB GL_EXT_texture_sRGB_decode GL_EXT_timer_query GL_EXT_transform_feedback GL_EXT_vertex_array_bgra GL_APPLE_aux_depth_stencil GL_APPLE_client_storage GL_APPLE_element_array GL_APPLE_fence GL_APPLE_float_pixels GL_APPLE_flush_buffer_range GL_APPLE_flush_render GL_APPLE_object_purgeable GL_APPLE_packed_pixels GL_APPLE_pixel_buffer GL_APPLE_rgb_422 GL_APPLE_row_bytes GL_APPLE_specular_vector GL_APPLE_texture_range GL_APPLE_transform_hint GL_APPLE_vertex_array_object GL_APPLE_vertex_array_range GL_APPLE_vertex_point_size GL_APPLE_vertex_program_evaluators GL_APPLE_ycbcr_422 GL_ATI_blend_equation_separate GL_ATI_blend_weighted_minmax GL_ATI_separate_stencil GL_ATI_texture_compression_3dc GL_ATI_texture_env_combine3 GL_ATI_texture_float GL_ATI_texture_mirror_once GL_IBM_rasterpos_clip GL_NV_blend_square GL_NV_conditional_render GL_NV_depth_clamp GL_NV_fog_distance GL_NV_light_max_exponent GL_NV_texgen_reflection GL_SGI_color_matrix GL_SGIS_generate_mipmap GL_SGIS_texture_edge_clamp GL_SGIS_texture_lod
Found extension GL_ARB_framebuffer_object
Found extension GL_ARB_pixel_buffer_object
Found extension GL_ARB_sync
Found extension GL_ARB_texture_float
Found extension GL_ARB_texture_non_power_of_two
Found extension GL_ARB_texture_rectangle
Found extension GL_EXT_bgra
Found extension GL_EXT_framebuffer_blit
Found extension GL_EXT_framebuffer_multisample
Found extension GL_EXT_framebuffer_object
Found extension GL_EXT_texture_compression_s3tc
Found extension GL_EXT_texture_filter_anisotropic
Found extension GL_APPLE_client_storage
Comment 1 Jeff Gilbert [:jgilbert] 2012-05-29 10:17:02 PDT
Agreed, this should be hidden behind MOZ_GL_DEBUG.
Comment 2 John Drinkwater (:beta) 2012-06-01 08:24:31 PDT
Created attachment 629200 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

A stab at fixing this, my first patch.
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2012-06-01 08:38:18 PDT
Comment on attachment 629200 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

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

Don't re-read the env var everytime. We already read it, into a |mDebugMode| bitfield, so just check for mDebugMode != 0. If the env var hasn't been read yet when you need it, try shuffling things around.
Comment 4 John Drinkwater (:beta) 2012-06-01 10:43:18 PDT
Created attachment 629241 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

Changed per review comment, using sDebugMode.
Moved the sDebugMode mask code‐block further up in the InitWithPrefix function so it is usable.
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-06-01 11:04:58 PDT
Comment on attachment 629241 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

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

Just a couple of nits.

Also, I'm scared that this is playing with the value of |once| which could be tricky in the future. But this is OK as this whole function is a kludge and needs to be refactored completely.

::: gfx/gl/GLContext.cpp
@@ +328,4 @@
>      if (mInitialized) {
>  #ifdef DEBUG
>          static bool once = false;
> +        if ( sDebugMode != 0 ) {

No spaces around the condition inside of the (), and !=0 is useless, so I would write: if (sDebugMode).

@@ +526,5 @@
>      char *exts = strdup((char *)extensions);
>  
>  #ifdef DEBUG
>      static bool once = false;
> +    if ( sDebugMode != 0 ) {

Same.
Comment 6 John Drinkwater (:beta) 2012-06-01 12:22:33 PDT
Created attachment 629291 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

Fixed spacing and conditional.
Comment 7 Jesse Ruderman 2012-06-02 19:22:07 PDT
I still see the spew on Try:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12322065&tree=Try

Maybe the conditional is backwards?
Comment 8 John Drinkwater (:beta) 2012-06-06 10:49:32 PDT
(In reply to Jesse Ruderman from comment #7)
> I still see the spew on Try:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=12322065&tree=Try
> 
> Maybe the conditional is backwards?

Yeah gosh, thought I’d tested that. New patch coming.
Comment 9 John Drinkwater (:beta) 2012-06-06 10:55:13 PDT
Created attachment 630636 [details] [diff] [review]
Disable most WEBGL debug output unless env vars set.

Suppresses WebGL init, context messages and outstanding buffers.
Comment 10 Jesse Ruderman 2012-06-06 14:26:58 PDT
$ hg qimport ~/Desktop/bug-758844d.patch
$ hg qrefresh -u "John Drinkwater <john@nextraweb.com>" -m "Bug 758844 - Disable debug output unless MOZ_GL_DEBUG is defined. r?bjacob. (try: -b do -p all -u reftest,reftest-1 -t none)"
$ hg try

Results will appear at https://tbpl.mozilla.org/?rev=1fbecdfc6bfd&tree=Try
Comment 11 John Drinkwater (:beta) 2012-06-06 18:47:43 PDT
Try failed on Android due to unrelated changeset https://hg.mozilla.org/try/rev/762be4801739
Comment 12 Jeff Gilbert [:jgilbert] 2012-06-06 19:37:57 PDT
Comment on attachment 630636 [details] [diff] [review]
Disable most WEBGL debug output unless env vars set.

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

I think that's all of them, though I didn't actually check. But the ones you touch on here are done right.

Eventually, we'd like to drop the #ifdef DEBUG tags around things, since DebugMode() should get inlined to |false|, which should optimized out. However, we haven't check this, so are erring on the side of caution and leaving the #ifdefs for now.
Comment 13 Jesse Ruderman 2012-06-06 21:41:58 PDT
Is this good to push, or should we wait for bjacob to r+ the new patch as well?
Comment 14 John Drinkwater (:beta) 2012-06-11 03:43:15 PDT
I would have thought its good to go.
Comment 15 Benoit Jacob [:bjacob] (mostly away) 2012-06-11 05:47:46 PDT
Jeff's r+ is definitely all you need!
Comment 17 Graeme McCutcheon [:graememcc] 2012-06-12 03:06:29 PDT
https://hg.mozilla.org/mozilla-central/rev/29d9f71b24ac
Comment 18 Jesse Ruderman 2012-06-15 19:11:25 PDT
Bug 762265 effectively reverted this change without discussion.  I am unhappy about that.
Comment 19 Jeff Gilbert [:jgilbert] 2012-06-15 19:20:38 PDT
Ouch, yeah, bug collision. This was not at all the intent of bug 762265.
Comment 20 John Drinkwater (:beta) 2012-06-16 08:10:09 PDT
Created attachment 633815 [details] [diff] [review]
Revert some changes from Bug 762265

No worries, this should undo the change in Bug 762265.
Could you put this through a try please, as I cannot test output from EGL here.
Comment 22 Jeff Gilbert [:jgilbert] 2012-07-11 19:54:54 PDT
Created attachment 641318 [details] [diff] [review]
Quiet GFX spew except on the first run in DebugMode()

This ties in the above changes, does a tiny bit of cleanup, and tries to make the logic less convoluted.
Comment 24 Ed Morley [:emorley] 2012-07-18 05:57:20 PDT
https://hg.mozilla.org/mozilla-central/rev/0888a88ab4c7

Note You need to log in before you can comment on or make changes to this bug.