[MPlayer-dev-eng] Direct3D OSD discussion

[Reimar Döffinger]

Hello,
I do not like the design with the intermediate textures, also since its
current implementation squishes the OSD weirdly (try -aspect 3 or
-aspect 0.5), but I leave that for Georgi to decide.

> +    void *vertices = NULL;

Just give it the right type (vertex_t *) instead of casting it several
times later.

> +void vo_draw_alpha_l8a8(int w,int h, unsigned char* src, unsigned char *srca,
> +                        int srcstride, unsigned char* dstbase,int dststride)
> +{
> +    int y;
> +    for (y=0; y < h; y++) {
> +        register unsigned short *dst = (unsigned short*) dstbase;
> +        register int x;
> +        for (x=0; x < w; x++) {
> +            if (srca[x])
> +                dst[x] = ((srca[x]-1) << 8) | src[x];
> +        }

Using D3DBLEND_INVSRCALPHA and -srca[x] would mean
1) You can use memset again in draw_osd, which particularly on x64 will
be assembler-optimized and much faster
2) You can leave the "if (srca[x])" out which particularly on P4 systems
is likely to be faster (you will not save memory bandwidth most of the
time, you'd need 32 consecutive pixels to be transparent to save any
thing at all due to the cache).

> +    return;
> +}

That return is quite pointless.

> +    vo_draw_alpha_l8a8(w,h,src,srca,stride,
> +        (void *)((unsigned int)locked_rect.pBits+locked_rect.Pitch*y0+2*x0), locked_rect.Pitch);

NEVER EVER CAST a POINTER to INT! This will break on Windows x64. While
I am not yet able to build MPlayer for x64, this is only a matter of
time.
Not to mention that casting to char* has exactly the same effect and you
need one cast less overall.

> +        /* need to use a texture here */
> +        IDirect3DDevice9_SetRenderState(priv->d3d_device, D3DRS_LIGHTING, FALSE);
> +
> +        IDirect3DDevice9_SetSamplerState(priv->d3d_device, 0, D3DSAMP_MINFILTER, D3DTEXF_LINEAR );
> +        IDirect3DDevice9_SetSamplerState(priv->d3d_device, 0, D3DSAMP_MAGFILTER, D3DTEXF_LINEAR );

I guess these one could just be set right at initialization, seems
likely the will be appropriate for all cases.

> +        IDirect3DDevice9_SetStreamSource(priv->d3d_device, 0, priv->d3d_quad_vb, 0, sizeof(vertex_t) );
> +        IDirect3DDevice9_SetFVF(priv->d3d_device, D3DFVF_MY_VERTEX );
> +        IDirect3DDevice9_DrawPrimitive(priv->d3d_device, D3DPT_TRIANGLEFAN, 0, 2 );

I know that is the recommended way, but I think it would be simpler to
just use DrawPrimitiveUP here...
On the "opposite" side, just using D3DPT_TRIANGLESTRIP would only need
swapping two vertexes and continue to work if someone ever wants to use
D3D10... (opposite because I think DrawPrimitiveUP will not be available
in D3D10).

Greetings,
Reimar Doeffinger