anholt / libepoxy

Epoxy is a library for handling OpenGL function pointer management for you
Other
680 stars 161 forks source link

macOS epoxy_has_gl_extension() only working with legacy OpenGL #219

Closed dqh-au closed 4 years ago

dqh-au commented 4 years ago

I've been playing with trying to understand why VICE Gtk3 performs so bad on macOS vs linux and Windows.

I discovered that VICE works great if Gdk doesn't ask for OpenGL 3.2 or 4.1, and gets the legacy renderer instead. Host CPU performance goes from about 75% to 45%.

When running with gl 2.1, epoxy_has_gl_extension() reports support for GL_ARB_texture_non_power_of_two, GL_ARB_texture_rectangle and GL_EXT_framebuffer_blit. When running with gl 3.2 or 4.1, epoxy_has_gl_extension() does not detect support for these extensions.

I found that if I force the detection result for these extensions in Gdk, VICE performance is also great with the 3.2 and 4.1 renderers. So I assume there must be something wrong with epoxy_has_gl_extension() in this case. In fact, I just have to force detection of GL_EXT_framebuffer_blit to get the result i'm looking for

I intend to try to understand and fix this, but I thought i'd open an issue to track it.

ebassi commented 4 years ago

Epoxy has nothing to do with this.

The epoxy_has_gl_extension() function is just a thin wrapper around glGetString(GL_EXTENSIONS) which queries the GL implementation; so, if asking for a GL extension works with legacy GL contexts and does not work with non-legacy GL contexts, then it means Apple's GL driver returns different extensions depending on the version of the GL context currently in use.

I'd recommend opening an issue against GTK, specifying:

dqh-au commented 4 years ago

Ok, interesting. It seems weird to me that pretending that GL_EXT_framebuffer_blit was detected when it wasn't causes gdk/gtk to perform so much better though? Makes me wonder if the extension string Apple reports is slightly different for non legacy contexts. If this is the case, would it be valid to raise it here again given that Epoxy is there for detecting extensions?

dqh-au commented 4 years ago

So glGetString(GL_EXTENSIONS) returns NULL using non-legacy renderer. Using legacy I get:

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_debug_label GL_EXT_debug_marker GL_EXT_depth_bounds_test 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_NV_texture_barrier GL_SGI_color_matrix GL_SGIS_generate_mipmap GL_SGIS_texture_edge_clamp GL_SGIS_texture_lod

ebassi commented 4 years ago

f this is the case, would it be valid to raise it here again given that Epoxy is there for detecting extensions?

No. Libepoxy is not used to "detect" extensions: it's used to call GL API to detect extensions, which means:

Epoxy is just a minimal wrapper around GL; it has no logic of its own.

It seems weird to me that pretending that GL_EXT_framebuffer_blit was detected when it wasn't causes gdk/gtk to perform so much better though?

It's better to not "pretend" anything.

GL works by feature negotiation: you ask what extensions it supports, and then use the API provided by and extension only if it's available (otherwise you'll get an abort() because the symbol you called does not exist).

The functionality provided by GL_EXT_framebuffer_blit should be part of GL 3.2+ cores; GL_ARB_texture_non_power_of_two is part of GL 2.0; and GL_ARB_texture_rectangle is generally deprecated. Ideally, the GL implementation inside the Quartz backend for GDK could hard code those extensions assuming that the version of the GL context retrieved matches the required versions of the extensions. Again, this is a bug for GTK, not Epoxy.

ebassi commented 4 years ago

So glGetString(GL_EXTENSIONS) returns NULL using non-legacy renderer

This could be a bug in GDK. Please file one.

dqh-au commented 4 years ago

glGetString(GL_EXTENSIONS) is deprecated as of OpenGL 3.0, according to discussion at https://community.khronos.org/t/gl-extensions-replacement/55542 . They're saying that enumeration is the new way of doing things. Genuinely sorry to labour the point but it really seems like epoxy_has_gl_extension() could support the enumeration method of querying reported support, at least on macOS?

Anyway i've made my case as completely as I can, will lodge bug / patch against GDK if you don't agree. Thanks for reading.

(The 'pretending' support for GL_EXT_framebuffer_blit was just a hack to see what would happen)

ebassi commented 4 years ago

Then feel free to open a pull request that makes epoxy_has_gl_extension() use glGetStringi() when the current GL context has a version >= 32, while maintaining the existing API.

dqh-au commented 4 years ago

Great, thanks and I will.

dqh-au commented 4 years ago

It turns out epoxy already implements enumeration for GL >= 30, it is not using glGetString(GL_EXTENSIONS). I believe this is a GDK bug after all, as you pointed out GL_ARB_texture_non_power_of_two is core GL 2.0 and GL_EXT_framebuffer_blit is core 3.0, and from what i'm reading these don't need to be reported as 'extensions' with these contexts.

dqh-au commented 4 years ago

Struggling to find an authoritative source for which gl version an extension becomes core. Incorporating that information into epoxy would make epoxy_has_gl_extension() do what GDK and probably other users of the library are expecting. Would you consider such a patch, if I could find a definitive source of when extensions become core? Probably a lot of work though.

dqh-au commented 4 years ago

There is gl.xml https://www.khronos.org/registry/OpenGL/xml/ that appears to contain authoritative data on which extensions are core in which gl version.

What if I wrote a python script that generated lookup tables for epoxy, and modified epoxy_has_gl_extension() to check core functionality based on epoxy_gl_version() before looking at the reported extensions? I am wondering if macOS is unique in not reporting previous-but-now-core extensions, there might be other drivers out there doing this.

My immediate problem with Gtk / Gdk could be fixed with a Gdk patch incorporating gl version checks into the feature detection code, but it seems to be that the larger community would be better served if epoxy transparently handled this.

nacho commented 4 years ago

I recall debugging this some time ago and afaik while GL_ARB_texture_non_power_of_two is part of core GL I think it was actually not supported by the opengl from OSX...

dqh-au commented 4 years ago

It's certainly reported as available when you get an OpenGL 2.1 context on macOS, just not when you get a 3.2 or 4.1 context. Is it possible your memory is simply based on glGetStringi(GL_EXTENSIONS, i) not reporting support for GL_ARB_texture_non_power_of_two? Because that's the core issue here - I think core features are not being reported by the GL_EXTENSIONS mechanism on macOS when many people expect them to be.

One thing I know is that GtkGlArea performs a LOT better for VICE on macOS if I:

  1. Change GDK to use the GL 2.1 context (which reports GL_EXT_framebuffer_blit), OR
  2. Change GDK to think that GL_EXT_framebuffer_blit is reported by epoxy_has_gl_extension() with a 3.2 or 4.1 context.

To me the simplest explanation is not that macOS has an incomplete GL feature set for a given api level, but that macOS doesn't report features as extensions once they are part of the core api for the created context, which kind of makes sense.

anholt commented 4 years ago

It is perfectly reasonable for an implementation to not report extensions whose functionality was also integrated into the core. It's a bit silly, and Mesa decided to expose those extensions anyway due to the issue that your app is seeing, but ultimately it's up to the app.

GDK should be checking if is_desktop_gl && gl_version >= 2.0 and assuming NPOT textuers, and desktop && version >= 3.0 for framebuffer_blit. The GL spec PDFs have have sections at the end describing the changes between versions and what functionality got folded in.

If we extended epoxy to expose extra extensions when the core version covers it, we would have to be very careful. Often extension functionality got slightly trimmed down in being folded into the core, so just saying that core supports them would break applications.

dqh-au commented 4 years ago

Thanks for explaining that. Perhaps there's space in the world for separate library that automates the version checks and then calls into Epoxy as needed.. At any rate, i'll be submitting a GDK patch as you describe and hopefully bring Gtk 3 towards being as viable for VICE as it is on Linux and WIndows.