The default bug view has changed. See this FAQ.

Quiet graphics code during startup

RESOLVED FIXED in mozilla17

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: beta)

Tracking

(Blocks: 1 bug)

Trunk
mozilla17
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

5 years ago
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
Agreed, this should be hidden behind MOZ_GL_DEBUG.
(Assignee)

Comment 2

5 years ago
Created attachment 629200 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

A stab at fixing this, my first patch.
Attachment #629200 - Flags: review?(bjacob)
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.
Attachment #629200 - Flags: review?(bjacob) → review-
(Assignee)

Comment 4

5 years ago
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.
Attachment #629200 - Attachment is obsolete: true
Attachment #629241 - Flags: review?(bjacob)
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.
Attachment #629241 - Flags: review?(bjacob) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 629291 [details] [diff] [review]
Disable debug output unless MOZ_GL_DEBUG is defined

Fixed spacing and conditional.
Attachment #629241 - Attachment is obsolete: true
Attachment #629291 - Flags: review+
(Reporter)

Comment 7

5 years ago
I still see the spew on Try:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12322065&tree=Try

Maybe the conditional is backwards?
(Assignee)

Comment 8

5 years ago
(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.
(Assignee)

Comment 9

5 years ago
Created attachment 630636 [details] [diff] [review]
Disable most WEBGL debug output unless env vars set.

Suppresses WebGL init, context messages and outstanding buffers.
Attachment #629291 - Attachment is obsolete: true
Attachment #630636 - Flags: review?
(Reporter)

Updated

5 years ago
Attachment #630636 - Flags: review? → review?(bjacob)
(Reporter)

Comment 10

5 years ago
$ 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
(Reporter)

Updated

5 years ago
Assignee: nobody → john
(Assignee)

Comment 11

5 years ago
Try failed on Android due to unrelated changeset https://hg.mozilla.org/try/rev/762be4801739
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.
Attachment #630636 - Flags: review+
(Reporter)

Comment 13

5 years ago
Is this good to push, or should we wait for bjacob to r+ the new patch as well?
(Assignee)

Comment 14

5 years ago
I would have thought its good to go.
Keywords: checkin-needed
OS: Mac OS X → All
Jeff's r+ is definitely all you need!
Attachment #630636 - Flags: review?(bjacob)
(Reporter)

Comment 16

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/29d9f71b24ac
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/29d9f71b24ac
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
(Reporter)

Comment 18

5 years ago
Bug 762265 effectively reverted this change without discussion.  I am unhappy about that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ouch, yeah, bug collision. This was not at all the intent of bug 762265.
Depends on: 762265
(Assignee)

Comment 20

5 years ago
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.
Attachment #633815 - Flags: review?
(Reporter)

Comment 21

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=996ba2a2ee4b
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.
Attachment #633815 - Attachment is obsolete: true
Attachment #633815 - Flags: review?
Attachment #641318 - Flags: review?(bjacob)
Attachment #641318 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0888a88ab4c7
Status: REOPENED → ASSIGNED
Target Milestone: mozilla16 → mozilla17
https://hg.mozilla.org/mozilla-central/rev/0888a88ab4c7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.