[PATCH xserver 3/3] render: Fix default picture format initialization

Adam Jackson ajax at redhat.com
Fri Feb 23 16:36:32 UTC 2018


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


More information about the xorg-devel mailing list