[MPlayer-dev-eng] [PATCH] Direct3D vo performance increase

[Reimar Döffinger]

On Sat, Nov 22, 2008 at 07:55:43PM +0200, Georgi Petrov wrote:
> Just one question - should me eliminated the "goto"?

Only if it makes the code not harder to read. So IMO in this case no.
Secondly, there are some unrelated things in there, like the Dst -> dst
renaming, they must be split at some point.
Also, IMO do not use a special is_surface_locked but just set
locked_rect.pBits to NULL after unlock and check that.

> @@ -139,7 +140,7 @@
>          priv->fs_panscan_rect.right  = priv->src_width - border;
>      } else {
>          priv->fs_movie_rect.left     = (vo_dwidth - scaled_width) / 2;
> -        priv->fs_movie_rect.right    = priv->fs_movie_rect.left + scaled_width;
> +    priv->fs_movie_rect.right  = priv->fs_movie_rect.left + scaled_width;
>      }
>      if (scaled_height > vo_dheight) {
>          int border = priv->src_height * (scaled_height - vo_dheight) / scaled_height;
> @@ -148,7 +149,7 @@
>          priv->fs_panscan_rect.bottom = priv->src_height - border;
>      } else {
>          priv->fs_movie_rect.top      = (vo_dheight - scaled_height) / 2;
> -        priv->fs_movie_rect.bottom   = priv->fs_movie_rect.top + scaled_height;
> +    priv->fs_movie_rect.bottom = priv->fs_movie_rect.top + scaled_height;
>      }
>  
>      mp_msg(MSGT_VO,MSGL_V,

These do not look intentional. It usually is a good idea to look at your
patches before sending them.
And as said, moving the BeginScene and changing locking should be done
in two steps. And since I think I forgot to answer that: The reason why
is because it is not _necessary_ to do it in one patch.
Large patches are always bad: hard to review, harder to avoid
side-effects, hard to debug/fix if they cause a regression, doubly so if
the regression is not reproduceable for any of the developers.
In case of a performanc patch not splitting also means that hardly
anyone will test the changes independently, or can any of you promise
that both will increase performance on all hardware and driver
combinations?

Greetings,
Reimar Döffinger