[Mesa-dev] gallium scaled types

[Jose Fonseca]

----- Original Message -----
> On Tue, Sep 13, 2011 at 8:45 PM, Jose Fonseca <jfonseca at vmware.com>
> wrote:
> >
> >
> > ----- Original Message -----
> >> José, Roland,
> >>
> >> there is a problem with the proposed approach of having the
> >> convert_to_float flag. That flag might not be supported for
> >> certain
> >> integer formats. We would need to update some more places, e.g.
> >> is_format_supported:
> >>
> >> boolean (*is_format_supported)( struct pipe_screen *,
> >>                                    enum pipe_format format,
> >>                                    enum pipe_texture_target
> >>                                    target,
> >>                                    unsigned sample_count,
> >>                                    unsigned bindings,
> >>                                    boolean
> >>                                    can_be_converted_to_float
> >>                                    );
> >
> >
> >
> >> See the last parameter. Ugly, isn't it? There is no other way
> >> around
> >> it.
> >
> > AFAIK, is_format_supported() was never used for vertex formats,
> > just textures. And given that textures can only be sampled without
> > implicit conversion (i.e., the "pure" vs scaled integers doesn't
> > apply), I see no need to add can_be_converted_to_float to
> > is_format_supported().
> >
> > I think what we need a is_vertex_format_supported(), or something
> > like that.
> 
> We have already been using is_format_supported for vertex buffers
> since ARB_half_float_vertex.
> 
> >
> >> The translate module also needs to distinguish between scaled and
> >> integer types. Translate currently doesn't support integers, only
> >> scaled. The code in Translate would either have to look like this:
> >>
> >> case PIPE_FORMAT_R32_SSCALED:
> >>    return is_int ? &emit_R32_SINT : &emit_32_SSCALED;
> >>
> >> Or like this:
> >>
> >> case PIPE_FORMAT_R32_SINT:
> >>    return is_scaled ? &emit_R32_SSCALED : &emit_32_SINT;
> >
> > No, the translate module doesn't need to change like that.
> >
> > What the translate module needs to do is ensure that
> > PIPE_FORMAT_xxxx_SINT -> PIPE_FORMAT_xxx_SINT doesn't get
> > converted to a float intermediate, but into an integer
> > intermediate instead.
> >
> > When the translate module is converting from PIPE_FORMAT_xxx_SINT
> > -> PIPE_FORMAT_xxx_FLOAT or PIPE_FORMAT_xxx_FLOAT ->
> > PIPE_FORMAT_xxx_SINT, telling that you want "pure integers" is
> > pointless!
> >
> > So, yes, the translate module's _internals_ needs to change, but
> > its interface needs _no_ change at all. In summary, the "is_int"
> > flag you mention can be determined from the current inputs.
> >
> >> The fairly new extension GL_ARB_vertex_type_2_10_10_10_rev also
> >> adds
> >> new scaled types and NOT integers (see Issue 7 of the
> >> specification).
> >
> > It looks you need ta PIPE_FORMAT_R10G10B10A2_SNORM and a
> > PIPE_FORMAT_R10G10B10A2_SINT (previously known as _SCALED) to
> > acommodate the different semantics for different legacy
> > attributes.
> 
> You are talking about apples and oranges. If
> PIPE_FORMAT_R10G10B10A2_SSCALED is redundant with
> PIPE_FORMAT_R10G10B10A2_SINT, then also PIPE_FORMAT_R10G10B10A2_SNORM
> is redundant with PIPE_FORMAT_R10G10B10A2_SINT. If we shouldn't have
> a
> set of types for convert-to-float-unnormalized, then obviously we
> shouldn't have a set of types for convert-to-float-normalized either.

On the contrary, I think that putting norm and scaled/int in the same sack is talking apples and oranges...   Normalization, like fixed-point integers, affects the interpretation of the 32bit integer in memory, namely the scale factor that it should be multiplied. Whereas  the only difference between _SSCALED and _SINT is the final data type (int vs float) -- the value is exactly the same (modulo rounding errors).

The pure vs non-pure integer is really a "policy" flag, which means please do not implicetly convert this to a float at any point of the vertex pipeline. And I believe that policy flags should be outside enum pipe_format.

> We need a state variable somewhere that can take these 3 values:
> - NORM
> - SCALED
> - INT
> And not having one half of the type encoded in a pipe format, and
> another half encoded in some CSO.

I already agreed we need the state on pipe_vertex_element and pipe_context::is_format_supported.  But we do not need such state on translate module.


For illustration I've added the snorm case to my earlier table, and added a new column to show the intermediate type that should be used internally by translate:

   translate            translate           intermediate
   input format         output format       datatype      notes
   -------------------+--------------------+------------+-------------------------
   R32G32B32A32_FLOAT | R32G32B32A32_FLOAT | float      |
   R32G32B32A32_FLOAT | R32G32B32A32_INT   | any        |
   R32G32B32A32_FLOAT | R32G32B32A32_SNORM | float      |
   R32G32B32A32_INT   | R32G32B32A32_FLOAT | any        | scaled, aka as not "pure integer"
   R32G32B32A32_INT   | R32G32B32A32_INT   | int        | unscaled, aka "pure integer"
   R32G32B32A32_SNORM | R32G32B32A32_FLOAT | float      |
   R32G32B32A32_SNORM | R32G32B32A32_INT   | any        | massive data loss (it will only give -1, 0, or -1)

where "any" means either float or int.  As you can see, the translate module can choose the appropriate data type that guarantees no precision loss of integer values, both on scaled and "pure integer" case, without any additional policy on the pipe_format enum.


Anyway, I think I've said everything I could say about this topic.  But I'm not actively working on gallium ATM so I don't fell authority to mandate anything.  I already spent too much time researching specs and code, quoting and writing up tables, to give you the best advice I can, but I confess it's getting tiring now.   Maybe Keith or Brian have an opinion and can weigh in on this matter.  If not, then my suggestion is for those of you working on this to decide among yourselves what course of action you want, and just do it.

After all, the important is getting those new features in -- it is not always possible to known what's best beforehand but it always possible to cleanup later -- and I think I'd be only wasting everybody's time by pursuing this.


Jose