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

[Max Reitz <mreitz@redhat.com>]

On 2015-01-20 at 06:00, Gerd Hoffmann wrote:
>    Hi,
>
>>> +static GLchar texture_blit_vert_src[] =
>>> +    "\n"
>>> +    "#version 300 es\n"
>>> +    "\n"
>>> +    "in vec2  in_position;\n"
>>> +    "in vec2  in_tex_coord;\n"
>> You could calculate the texture coordinate from the position in the
>> shader, but this is mostly my premature optimization instinct kicking in
>> instead of a real performance difference considering how few vertices
>> there are in this case.
> Still makes sense.  Done.
>
>>> +    "out vec2 ex_tex_coord;\n"
>>> +    "\n"
>>> +    "void main(void) {\n"
>>> +    "    gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n"
>> vec4(in_position, 0.0, 1.0) *cough*
> Done.
>
>> This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
>> For GL_TRIANGLE_STRIP, you first define one whole triangle (by
>> specifying each of the three vertices) and after that, each vertex
>> results in a new triangle drawn (whose two other vertices are the two
>> vertices preceding the last one).
> Thanks for the nice description.
>
> So the trick to get it done with only four vertexes is to put the
> correct points (which are going to be shared by both triangles) into the
> middle.  I've played around with it a bit without success (and before my
> new opengl book arrived), and this 6-point version worked ...

Hm, I did write a working solution in my reply, didn't I? It was: { -1, 
-1,   1, -1,   -1, 1,   1, 1 } for in_position[] and { 0, 1, 1, 1,   0, 
0,   1, 0 } for in_tex_coord[]. At least it worked for me.

You haven't (explicitly) enabled face culling, but remember that while 
normally counter-clockwise faces are backfaces and thus culled, the 
direction is inverted for every triangle; so the first triangle needs to 
be CCW, the second CW, the third CCW again, and so on. Maybe that's why 
it didn't work for you...

>>> +    glUseProgram(gls->texture_blit_prog);
>>> +    l_position = glGetAttribLocation(gls->texture_blit_prog, "in_position");
>>> +    l_tex_coord = glGetAttribLocation(gls->texture_blit_prog, "in_tex_coord");
>>> +    glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
>>> +    glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
>> I think it is disallowed to refer to non-OpenGL buffers here in the core
>> profile. The 4.4 core specification states (section 10.3, vertex
>> arrays): "Vertex data are placed into arrays that are stored in the
>> server’s address space"; the 4.4 compatibility specification states:
>> "Vertex data may also be placed into arrays that are stored in the
>> client’s address space (described here) or in the server’s address space".
> Got this from gles sample code, so that should be ok ;)
>
> I've also seen code explicitly storing vertex data in a buffer, which I
> assume is more efficient, but I decided to not care for now, especially
> given the small number of vertexes we are processing here.
>
>>> +    return gl_create_link_program(vert_shader, frag_shader);
>> Minor thing: You are free to delete the shaders after they have been
>> linked into the program (because the references will be lost when this
>> function returns).
> Done.
>
>>> +    switch (surface->format) {
>>> +    case PIXMAN_BE_b8g8r8x8:
>>> +    case PIXMAN_BE_b8g8r8a8:
>>> +        surface->glformat = GL_BGRA_EXT;
>> If you want to avoid the _EXT, you could use GL_RGBA here and
>> texture().bgra in the fragment shader. However, this would require a
>> different shader for the 32 bit and the 16 bit formats (because the 16
>> bit format has the right order, apparently).
> At least it worked in testing.
>
>> I haven't been able to really test 16 bit mode (forcing 16 bit mode in
>> hw/display/vga.c doesn't count...), so I'm trusting you this works.
> -kernel /boot/vmlinux-$whatever -append vga=0x314
>
> Makes the kernel run vesafb with 800x600 @ 16bpp, and thanks to the
> little friendly penguin in the upper left corner (CONFIG_LOGO=y) you can
> easily spot colors being messed up.

Thanks!

Max