[Mesa-dev] [RFC PATCH] gallium: add interface for EQAA

[Nicolai Hähnle]

On 01.05.2018 16:48, Roland Scheidegger wrote:
>> -**nr_samples** the nr of msaa samples. 0 (or 1) specifies a resource
>> -which isn't multisampled.
>> +**nr_samples**: For Z/S, this is the number of samples. For color, if EQAA
>> +is unsupported, this is the number of both coverage samples and color samples.
>> +If EQAA is supported, this is the number of coverage samples. 0 and 1
>> +specify a resource which isn't multisampled.
> I think you should keep nr_samples alone, and re-change the meaning to
> actually be "real" samples with all associated data (which is what
> everybody expects with msaa).
> 
>> +
>> +**nr_color_samples**: This is the number of color samples for EQAA, while
>> +``nr_samples`` is the number of coverage samples. If the format is Z/S,
>> +``nr_color_samples`` is ignored. Constraints:
>> +* ``nr_color_samples`` must not be greater than ``nr_samples``.
>> +* If ``nr_color_samples`` is equal to ``nr_samples``, it is called MSAA.
>> +* If ``nr_color_samples`` is less than ``nr_samples``, it is called EQAA.
>> +* If ``nr_color_samples`` is equal to 1, the behavior of the resolve blit is
>> +driver-dependent.
>>   
> Hence instead use something like "total_samples" or "coverage samples"
> here (albeit the latter could be a bit confusing, unclear if it means
> all samples or just the extra ones).
> If you'd use something like "number of additional coverage samples"
> instead, then you could even leave all state trackers alone, since 0
> would automatically mean ordinary msaa without crazy stuff.

Having the new variable be about coverage samples makes sense.

I'm not convinced that having it be the number of *additional* samples 
is so convenient. At least in our hardware, there tend to be fields that 
encode the log of total samples of each type, so having a total number 
of coverage samples seems more convenient. That said, it's a bit 
bike-shedding.

Cheers,
Nicolai


> And fwiw I'd stick to the CSAA name - coverage at least has some
> meaning, whereas "enhanced" (which is what the "E" in EQAA is for
> according to some old articles) really is just completely meaningless
> marketing term. There really isn't any difference, ok the ability to use
> different amount of samples per rt might be something, but you can't
> expose this anyway (AMD didn't come up with an extension for this since
> the 7+ years it's supported (since NI), so I have some doubts of its
> usefulness - besides you could have another cap which then would say if
> that's valid or not if you really come up with an extension specifically
> for it.)
> Unless there's some non-technical reasons you can't use the term CSAA.
> 
> Roland
> 
> 
> 
>>   **usage** one of the :ref:`PIPE_USAGE` flags.
>>   
>>   **bind** bitmask of the :ref:`PIPE_BIND` flags.
>>   
>>   **flags** bitmask of PIPE_RESOURCE_FLAG flags.
>>   
>>   
>>   
>>   resource_changed
>> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
>> index c4ae0532060..97e1a3a3d42 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -783,20 +783,21 @@ enum pipe_cap
>>      PIPE_CAP_ALLOW_MAPPED_BUFFERS_DURING_EXECUTION,
>>      PIPE_CAP_POST_DEPTH_COVERAGE,
>>      PIPE_CAP_BINDLESS_TEXTURE,
>>      PIPE_CAP_NIR_SAMPLERS_AS_DEREF,
>>      PIPE_CAP_QUERY_SO_OVERFLOW,
>>      PIPE_CAP_MEMOBJ,
>>      PIPE_CAP_LOAD_CONSTBUF,
>>      PIPE_CAP_TGSI_ANY_REG_AS_ADDRESS,
>>      PIPE_CAP_TILE_RASTER_ORDER,
>>      PIPE_CAP_MAX_COMBINED_SHADER_OUTPUT_RESOURCES,
>> +   PIPE_CAP_EQAA_COLOR_SAMPLE_SUPPORT_MASK,
>>      PIPE_CAP_SIGNED_VERTEX_BUFFER_OFFSET,
>>      PIPE_CAP_CONTEXT_PRIORITY_MASK,
>>      PIPE_CAP_FENCE_SIGNAL,
>>      PIPE_CAP_CONSTBUF0_FLAGS,
>>      PIPE_CAP_PACKED_UNIFORMS,
>>   };
>>   
>>   /**
>>    * Possible bits for PIPE_CAP_CONTEXT_PRIORITY_MASK param, which should
>>    * return a bitmask of the supported priorities.  If the driver does not
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index 4dce399f848..4010b92e67b 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -503,41 +503,42 @@ struct pipe_box
>>      int16_t depth;
>>   };
>>   
>>   
>>   /**
>>    * A memory object/resource such as a vertex buffer or texture.
>>    */
>>   struct pipe_resource
>>   {
>>      struct pipe_reference reference;
>> -   struct pipe_screen *screen; /**< screen that this texture belongs to */
>>   
>>      unsigned width0; /**< Used by both buffers and textures. */
>>      uint16_t height0; /* Textures: The maximum height/depth/array_size is 16k. */
>>      uint16_t depth0;
>>      uint16_t array_size;
>>   
>>      enum pipe_format format:16;         /**< PIPE_FORMAT_x */
>>      enum pipe_texture_target target:8; /**< PIPE_TEXTURE_x */
>>      unsigned last_level:8;    /**< Index of last mipmap level present/defined */
>>      unsigned nr_samples:8;    /**< for multisampled surfaces, nr of samples */
>> +   unsigned nr_color_samples:8; /**< Number of color samples for EQAA. */
>>      unsigned usage:8;         /**< PIPE_USAGE_x (not a bitmask) */
>>   
>>      unsigned bind;            /**< bitmask of PIPE_BIND_x */
>>      unsigned flags;           /**< bitmask of PIPE_RESOURCE_FLAG_x */
>>   
>>      /**
>>       * For planar images, ie. YUV EGLImage external, etc, pointer to the
>>       * next plane.
>>       */
>>      struct pipe_resource *next;
>> +   struct pipe_screen *screen; /**< screen that this texture belongs to */
>>   };
>>   
>>   
>>   /**
>>    * Transfer object.  For data transfer to/from a resource.
>>    */
>>   struct pipe_transfer
>>   {
>>      struct pipe_resource *resource; /**< resource to transfer to/from  */
>>      unsigned level;                 /**< texture mipmap level */
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.