Re: [Qemu-devel] [PATCH 2/4] console: add opengl rendering helper functions

[Max Reitz]

On 2015-01-16 at 04:21, Gerd Hoffmann wrote:
>    Hi,
>
> > > +bool console_gl_check_format(DisplayChangeListener *dcl,
> > > +                             pixman_format_code_t format)
> > > +{
> > > +    switch (format) {
> > > +    case PIXMAN_x8r8g8b8:
> > > +    case PIXMAN_a8r8g8b8:
> > > +    case PIXMAN_r5g6b5:
> > > +        return true;
> > > +    default:
> > > +        return false;
> > > +    }
> > > +}
> > What is this function supposed to be used for?
> Inflight change, there is a patch series on the list adding a
> check_format callback to DisplayChangeListenerOps, for format
> negotiation.

OK.

> > > +
> > > +void surface_gl_create_texture(DisplaySurface *surface)
> > > +{
> > > +    switch (surface->format) {
> > > +    case PIXMAN_x8r8g8b8:
> > > +    case PIXMAN_a8r8g8b8:
> > > +        surface->glformat = GL_BGRA;
> > Why does the format code seem to imply ARGB order but you're using the
> > exact opposite? I could imagine something like endianness being the
> > reason, but you're using RGB below where the format code says exactly that.
> endianness indeed.  pixman formats are native endian, so this actually
> is bgra ordering in memory.  While looking at it:  I guess GL_BGRA is
> fixed byte ordering?

Yes, I think so.

> So this probably needs fixing to work correctly on
> bigendian machines ...

Probably, right.

> > Discarding the alpha channel is intentional, I suppose?
> Yes.
>
> > > +                 surface_width(surface),
> > > +                 surface_height(surface),
> > > +                 0, surface->glformat, surface->gltype,
> > > +                 surface_data(surface));
> > Is surface_stride(surface) specified to be surface_width(surface) *
> > bytes_per_pixel (I don't know pixman so I don't know)?
> Usually this is the case, but there can be exceptions.  Hmm, can I
> explicitly pass the stride?

The internet™ tells me about glPixelStorei(GL_UNPACK_ROW_LENGTH, 
$pixels_per_row) and glPixelStorei(GL_UNPACK_ALIGNMENT, 
$row_alignment_in_bytes) (which need to be called before using 
glTexImage2D() or glTexSubImage2D()). You may want to use it for 
surface_gl_update_texture(), too (if that isn't too much work).

Max