[MPlayer-dev-eng] [PATCH] - No Reset of D3D device on resize

[Georgi Petrov]

>> Hey, did you forget the patch. Just checking :)
>
> No, but I had not much time and there were places that looked very
> suspicious.

Ok, I understand that you don't have much time. I develop for MPlayer
in my free time as well, but if you find anything suspicious, please
let me know when you discover it. I suppose you didn't check the
patch, decided it was suspicious and left it behind without notifying
me, but you just didn't have time to check it at all :)

> You are duplicating the handling of d3d_surface and d3d_backbuf
> deallocation in three places which is already quite ugly.

No, they shouldn't go hand in hand anymore. Now we have 3 "types" of
surfaces/textures:

1. The offscreen surface.
2. The backbuffer surface.
3. The OSD textures.

The first one is never recreated except when we do a device reset,
because it's always equal to the movie's dimensions. To do a Reset,
all allocated resources should be freed. Otherwise we would keep it as
well.

The backbuffer is never recreated except when growing the window
larger than fullscreen (the backbuffer is now set to the fullscreen
initially).

The OSD textures should always match current window dimensions, so
they are freed/recreated on each resize.

As you see, we can't handle all those three things the same way,
because handling them is required at different "times" now.

> But in addition you do not release them at all in the configure()
> function any more, and you do not release the backbuffer
> in the reconfigure function.

I fixed both problems. I attach the updated patch.

> Now, I think there is no need to release them, since the whole
> d3d_device is released but then
> 1) should that be changed in a different patch
> 2) there is no reason to release the surface/osd textures either
>

Huh, isn't this part of this patch, because it changes the way all
surfaces/textures creation/destruction is handled? And I suppose it's
always better to release something explicitly than leave it to the D3D
runtime.

> Lastly, in resize_d3d you would first call
> check_d3d_backbuffer_dimensions which may do a Reset and then destroy
> and reallocate the OSD textures. I would have to re-read the Reset
> description to see if that is ok, but it sure seems ugly to me,
> IMO it should be a simple
> if (vo_dwidth > priv->cur_backbuf_width || vo_dheight ...) {
>  update cur_backbuf_*
>  same code as the current one
> } else {
>  destroy_d3d_osd_textures();
>  create_d3d_osd_textures();
> }
>

>From MSDN:

Calling IDirect3DDevice9::Reset causes all texture memory surfaces to
be lost, managed textures to be flushed from video memory, and all
state information to be lost. Before calling the
IDirect3DDevice9::Reset method for a device, an application should
release any explicit render targets, depth stencil surfaces,
additional swap chains, state blocks, and D3DPOOL_DEFAULT resources
associated with the device.

So we should free all resources before Reset, just as we did on each
resize before this patch. The point of
"check_d3d_backbuffer_dimensions" in the beginning of d3d_resize() is
to do a Reset with a new (larger) backbuffer only if necessary (which
includes destroying all D3D resources and reallocating them again).
Hopefully "check_d3d_backbuffer_dimensions" won't do anything (the new
dimensions will be smaller than the backbuffer) and it will just quit.
You see, at the beginning of the function we have exactly what do you
propose:

    if (vo_dwidth <= priv->cur_backbuf_width &&
        vo_dheight <= priv->cur_backbuf_height)
        return 1;

So i exits if the new dimensions are smaller than the backbuffer, or
does a full Reset if they are larger.

I attach the updated patch.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: smooth_resize2.txt
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20081224/91cf0ef5/attachment.txt>