On Thu, 2018-02-22 at 16:52 -0800, Keith Packard wrote:

> >          switch (bpp) {
> >          case 16:
> >              /* depth 12 formats */
> > -            if (pDepth->depth >= 12) {
> > -                addFormat(formats, &nformats, PICT_x4r4g4b4, 
> > pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_x4b4g4r4, 
> > pDepth->depth);
> > -            }
> > +            addFormat(formats, &nformats, PICT_x4r4g4b4, 12);
> > +            addFormat(formats, &nformats, PICT_x4b4g4r4, 12);
> >              /* depth 15 formats */
> > -            if (pDepth->depth >= 15) {
> > -                addFormat(formats, &nformats, PICT_x1r5g5b5, 
> > pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_x1b5g5r5, 
> > pDepth->depth);
> > -            }
> > +            addFormat(formats, &nformats, PICT_x1r5g5b5, 15);
> > +            addFormat(formats, &nformats, PICT_x1b5g5r5, 15);
> >              /* depth 16 formats */
> > -            if (pDepth->depth >= 16) {
> > -                addFormat(formats, &nformats, PICT_a1r5g5b5, 
> > pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_a1b5g5r5, 
> > pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_r5g6b5, pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_b5g6r5, pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_a4r4g4b4, 
> > pDepth->depth);
> > -                addFormat(formats, &nformats, PICT_a4b4g4r4, 
> > pDepth->depth);
> > -            }
> > +            addFormat(formats, &nformats, PICT_a1r5g5b5, 16);
> > +            addFormat(formats, &nformats, PICT_a1b5g5r5, 16);
> > +            addFormat(formats, &nformats, PICT_r5g6b5, 16);
> > +            addFormat(formats, &nformats, PICT_b5g6r5, 16);
> > +            addFormat(formats, &nformats, PICT_a4r4g4b4, 16);
> > +            addFormat(formats, &nformats, PICT_a4b4g4r4, 16);
> 
> You can't just blindly add formats that the driver might not
> support. Depth really means the bits supported by the driver; bits
> outside of depth may not be present in the hardware.

This is at the bottom of fbPictureInit. This is code that every driver
already runs. The loop will find that a pixmap of depth 16 has 16 bits
per pixel, and since that's larger than 12, it will add x4r4g4b4.

You might be right that we _should not_ expose formats not present in
the hardware, though Render has kinda already lost that fight by making
a1 and a4 mandatory. But the Render code today, and forever, is making
the assertion that the code above it _will_ handle these formats.

- ajax
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to