On 01/20/2016 12:22 AM, Nanley Chery wrote: > On Tue, Jan 19, 2016 at 05:41:40PM +0100, Eduardo Lima Mitev wrote: >> Hello, >> >> This is an RFC series adding support for the ARB_internalformat-query2 >> extension: >> >> https://www.opengl.org/registry/specs/ARB/internalformat_query2.txt >> >> The corresponding bug is being tracked at: >> >> https://bugs.freedesktop.org/show_bug.cgi?id=92687 >> >> Why is this an RFC series instead of a formal merge-able patch-set? >> >> Two reasons. Firstly, we are still polishing rough edges in some patches. >> However, the support is complete to the best of our knowledge. Most of the >> final changes we are making are improvements to particular query answers, >> thus contained inside specific blocks, so they don't affect the structure of >> the code. >> Secondly, we have been trying to get general feedback, and answers to some >> doubts we posted on bugzilla, without much success. So maybe an explicit RFC >> in the mailing list would bring more eyes to it. >> This is a rather large extension, with a long spec wording, certainly >> difficult to review. So we totally appreciate the effort and brain cycles of >> whoever takes on this. >> >> >> The patch-set is structured as follows: >> >> * Patches 01 to 10 sets up the stage to query2. It will add a new, generic >> driver hook that obsoletes QuerySamplesForFormat, which is removed. But it >> doesn't introduce anything related with query2 yet. >> >> * Patches 11 to 61 implement the different individual queries from query2 >> extension in the frontend (mesa/main), adding validation and helper >> functions as needed. > > There are a number of patches in this block that access the > ctx->Extensions member directly. The new and safer way to check for > extension support is to use the auto-generated helper functions defined > in main/extensions.h. Given the extension you're interested in with > Name String, GL_<name_str>, you can see if it is enabled in the given > context by using the function, _mesa_has_<name_str>(ctx) > (e.g. GL_ARB_shader_image_load_store -> > _mesa_has_ARB_shader_image_load_store(ctx)). This does all the context > checking required to determine extensions support (desktop GL vs GLES, > driver support, and API version requirement). >
Hi Nanley, Yes, this is one of the things we will be fixing for the final series. When we started working on this, the new extensions API was still being discussed/reviewed on the mailing list. We thought then it was too premature to use, so we played safe and used the old way. Thanks a lot for the feedback! Eduardo > Regards, > Nanley > >> >> * Patches 62 to 63 activates the extension on i965. Only the queries where >> the driver has something to return other than default value returned by the >> frontend, are explicitly added. >> >> >> Some implementation notes: >> >> * All the extension's frontend code is in main/formatquery.c, as it was >> before for query1. Only that it also handles query2 now. >> >> * As commented above, a new driver hook 'QueryInternalFormat' was added, >> replacing the previous one 'QuerySamplesForFormat'. >> >> * A fallback, generic function _mesa_query_internal_format_default() >> provides generic implementation and sensible defaults for all queries, for >> drivers not implementing query2. Backends that only care about answering >> some queries, can call back this function for the other queries where a >> generic answer is ok. >> >> * For all pnames, the frontend code will do generic validation as per the >> spec: check GL profile, version, extensions. >> - If the frontend fails basic validation, it will give the corresponding >> negative answer, depending on the pname, without going to the driver. >> - If the frontend is fully qualified to provide an answer, it will (i.e, >> MAX_WIDTH, COLOR_COMPONENTS, etc). Otherwise it will call the driver hook >> (i.e, INTERNALFORMAT_PREFERRED). >> - For the cases where the query must return full support, caveat support, >> or no support; Mesa/main will always call the driver to decide between full >> or caveat support (and only answer directly in the case of no-support). >> >> * The last patches in the branch enable support for this extension in i965 >> backend (drivers/dri/i965/brw_formatquery.c). The backend code only handle >> queries where the answer is affected by driver-specific stuff. But by >> default, it calls back the frontend function with the default >> implementations. >> >> * The 64 bits version of the query introduced by this extension >> (GetInternalformati64v), was implemented as a wrapper around the 32 bits >> version. Since only one query really requires the 64 bits API >> (MAX_COMBINED_DIMENSIONS), we handle that pname as a special case. For the >> rest of queries, we just forward the call to the default, 32 bits version. >> >> >> A git tree of the series can be found at: >> >> https://github.com/Igalia/mesa/tree/internalformat-query2-rfc >> >> >> There is also a branch containing piglit tests for the extension, which my >> colleague Alejando will send to the piglit mailing list for feedback/review. >> >> >> cheers, >> Eduardo (on behalf of the team that worked on this) >> >> >> Alejandro PiƱeiro (9): >> mesa: Add dispatch and extension XML for GL_ARB_internalformat_query2 >> mesa/main: not fill mesa_error on >> _mesa_legal_texture_base_format_for_target >> mesa/formatquery: initial implementation for GetInternalformati64v >> mesa/formatquery: handle unmodified buffer for SAMPLES on the 64-bit >> query >> mesa/formatquery: support for IMAGE_FORMAT_COMPATIBILITY_TYPE >> main/formatquery: support for MAX_{WIDTH/HEIGHT/DEPTH/LAYERS} >> mesa/formatquery: support for MAX_COMBINED_DIMENSIONS >> mesa/texparam: make public target_allows_setting_sampler_parameters >> mesa/formatquery: added FILTER pname support >> >> Antia Puentes (36): >> mesa/main: Add extension tracking bit for ARB_internalformat_query2 >> mesa/formatquery: Added function to validate parameters >> mesa/formatquery: Added function to set 'unsupported' responses >> mesa/formatquery: Added a func to check if the <target> is supported >> mesa/formatquery: Added boilerplate code to extend GetInternalformativ >> mesa/main: Added empty skeleton of glGetInternalformati64v >> mesa/teximage: make public is_renderable_texture_format >> mesa/teximage: Make _mesa_format_no_online_compression public >> mesa/formatquery: Added func to check if the 'resource' is supported >> mesa/formatquery: Added a func to check <internalformat> supported >> mesa/formatquery: Added the INTERNALFORMAT_SUPPORTED <pname> query >> mesa/main: Make legal_get_tex_level_parameter_target public >> mesa/main: Extend _mesa_base_format_has_channel to accept new pnames >> mesa/main: Extend _mesa_get_format_bits to accept new pnames >> mesa/formatquery: Added INTERNALFORMAT_{X}_{SIZE,TYPE} <pname> queries >> mesa/formatquery: Added {COLOR,DEPTH,STENCIL}_COMPONENTS <pname> >> queries >> mesa/formatquery: Added {COLOR,DEPTH,STENCIL}_RENDERABLE <pname> >> queries >> mesa/genmipmap: Added a function to check if the target is valid >> mesa/genmipmap: Added a function to validate the internalformat >> mesa/formatquery: Added mipmap related <pname> queries >> mesa/formatquery: Added COLOR_ENCODING <pname> query. >> mesa/formatquery: Added SRGB_{READ,WRITE} <pname> queries >> mesa/formatquery: Added SRGB_DECODE_ARB <pname> query >> mesa/formatquery: Added queries related to texture sampling in shaders >> mesa/shaderimage: Make is_image_format_supported public >> mesa/formatquery: Added SHADER_IMAGE_{LOAD,STORE,ATOMIC} <pname> >> queries >> mesa/shaderimage: Added func to get the GL_IMAGE_CLASS from the format >> mesa/formatquery: Added queries related to image textures >> mesa/formatquery: Added simultaneous texture and depth/stencil queries >> mesa/formatquery: Added compressed texture related queries >> mesa/formatquery: Added CLEAR_BUFFER <pname> query >> mesa/textureview: Make _lookup_view_class public >> mesa/formatquery: Added texture view related queries >> mesa/formatquery: Added texture gather/shadow related queries >> mesa/formatquery: Added framebuffer renderability related queries >> i965: Enable the ARB_internalformat_query2 extension >> >> Eduardo Lima Mitev (18): >> mesa: Add QueryInternalFormat to device driver virtual table >> mesa: Add a default QueryInternalFormat() function for drivers >> i965: Add boilerplate function for QueryInternalFormat driver hook >> i965: Move brw_query_samples_for_format() to brw_queryformat.c >> i965/formatquery: Respond queries SAMPLES and NUM_SAMPLE_COUNTS >> st/format: Replace QuerySamplesForFormat by new QueryInternalFormat >> hook >> mesa/multisample: Check sample count using the new driver hook >> mesa/formatquery: Remove tracking of number of elements in the >> response >> mesa/formatquery: Use new driver hook QueryInternalFormat >> mesa: Completely remove QuerySamplesForFormat from driver func table >> mesa/formatquery: Added INTERNALFORMAT_PREFERRED pname >> mesa/teximage: add _mesa_is_cube_map_texture utility method >> mesa/formatquery: Add support for READ_PIXELS query >> mesa/formatquery: Add READ_PIXELS_FORMAT pname >> mesa/formatquery: Add READ_PIXELS_TYPE pname >> mesa/formatquery: Add (GET_)TEXTURE_IMAGE_FORMAT pnames >> mesa/formatquery: Add (GET_)TEXTURE_IMAGE_TYPE pnames >> i965/formatquery: Add support for INTERNALFORMAT_PREFERRED query >> >> src/mapi/glapi/gen/ARB_internalformat_query2.xml | 119 ++ >> src/mapi/glapi/gen/gl_API.xml | 2 +- >> src/mesa/drivers/common/driverfuncs.c | 2 +- >> src/mesa/drivers/dri/i965/Makefile.sources | 1 + >> src/mesa/drivers/dri/i965/brw_context.c | 40 +- >> src/mesa/drivers/dri/i965/brw_context.h | 5 + >> src/mesa/drivers/dri/i965/brw_formatquery.c | 131 ++ >> src/mesa/drivers/dri/i965/intel_extensions.c | 1 + >> src/mesa/main/dd.h | 26 +- >> src/mesa/main/extensions_table.h | 1 + >> src/mesa/main/formatquery.c | 1521 >> ++++++++++++++++++++-- >> src/mesa/main/formatquery.h | 9 + >> src/mesa/main/formats.c | 6 + >> src/mesa/main/genmipmap.c | 50 +- >> src/mesa/main/genmipmap.h | 6 + >> src/mesa/main/glformats.c | 12 + >> src/mesa/main/mtypes.h | 1 + >> src/mesa/main/multisample.c | 10 +- >> src/mesa/main/shaderimage.c | 58 +- >> src/mesa/main/shaderimage.h | 14 + >> src/mesa/main/tests/dispatch_sanity.cpp | 4 + >> src/mesa/main/teximage.c | 44 +- >> src/mesa/main/teximage.h | 14 +- >> src/mesa/main/texparam.c | 40 +- >> src/mesa/main/texparam.h | 7 + >> src/mesa/main/texstorage.c | 8 +- >> src/mesa/main/textureview.c | 12 +- >> src/mesa/main/textureview.h | 8 + >> src/mesa/state_tracker/st_cb_texture.c | 2 +- >> src/mesa/state_tracker/st_format.c | 38 +- >> src/mesa/state_tracker/st_format.h | 8 +- >> 31 files changed, 1961 insertions(+), 239 deletions(-) >> create mode 100644 src/mapi/glapi/gen/ARB_internalformat_query2.xml >> create mode 100644 src/mesa/drivers/dri/i965/brw_formatquery.c >> >> -- >> 2.5.3 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev