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

[Max Reitz <mreitz@redhat.com>]

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