<html>
    <head>
      <base href="https://bugs.freedesktop.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Unify the format conversion code"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=84566#c7">Comment # 7</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Unify the format conversion code"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=84566">bug 84566</a>
              from <span class="vcard"><a class="email" href="mailto:itoral@igalia.com" title="Iago Toral <itoral@igalia.com>"> <span class="fn">Iago Toral</span></a>
</span></b>
        <pre>(In reply to Jason Ekstrand from <a href="show_bug.cgi?id=84566#c6">comment #6</a>)
<span class="quote">> (In reply to Iago Toral from <a href="show_bug.cgi?id=84566#c5">comment #5</a>)
> > Jason, in your initial implementation of the master function you have this
> > assert:
> > 
> > if (src_array_format.as_uint && dst_array_format.as_uint) {
> >    assert(src_array_format.normalized == dst_array_format.normalized);
> >    (...)
> >    _mesa_swizzle_and_convert(dst, dst_gl_type, dst_array_format.num_channels,
> >                              src, src_gl_type, src_array_format.num_channels,
> >                              src2dst, normalized, width);
> > }
> > 
> > I think this assertion is wrong, since it prevents conversion from float
> > array types (which are being generated with normalized=false) to normalized
> > integer types. Something that is explicitly supported in
> > _mesa_swizzle_and_convert. I am hitting this assertion while converting from
> > RGB FLOAT to RGBA UBYTE for example.
> > 
> > Looking at the implementation of texstore_swizzle, which uses
> > _mesa_swizzle_and_convert (and is the path that the original code takes to
> > do this conversion in my example), I don't find this kind of checks. What it
> > does is this:
> > 
> > is_array = _mesa_format_to_array(dstFormat, &dst_type, &dst_components,
> >                                  rgba2dst, &normalized);
> > (...)
> > normalized |= !_mesa_is_enum_format_integer(srcFormat);
> > 
> > This will make it so that normalized is always set to true for float src
> > formats like RGB FLOAT. 
> > 
> > So assuming that this code is correct I think we would want to do the same
> > in the master function: remove the assert and set normalized to true if
> > either src or dst are normalized or if src is a float type.

> Yes, I believe that is correct.  While I implemented the master function, it
> didn't get much testing.  I'ts mostly just a prototype, so bugs like that
> aren't surprising.</span >

Yeah, I know, but since I don't really have any previous experience with the
formats code I prefer to double check my conclusions with you just in case I am
missing something or messing up.

BTW, Thanks for the fast reply!</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the QA Contact for the bug.</li>
      </ul>
    </body>
</html>