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

Reply via email to