[PATCH] d3d11: Add support for setting multiple viewports

[Henri Verbeet]

On 11 April 2018 at 09:36, Nikolay Sivov <nsivov at codeweavers.com> wrote:
> ---
>  dlls/d3d11/device.c            | 52 ++++++++++++------------
>  dlls/d3d11/tests/d3d11.c       |  3 --
>  dlls/d3d8/device.c             |  2 +-
>  dlls/d3d9/device.c             |  2 +-
>  dlls/ddraw/device.c            |  2 +-
>  dlls/wined3d/cs.c              | 34 ++++++++++------
>  dlls/wined3d/device.c          | 50 ++++++++++++++---------
>  dlls/wined3d/glsl_shader.c     | 13 ++++++
>  dlls/wined3d/state.c           | 92 +++++++++++++++++++++++++++++++-----------
>  dlls/wined3d/stateblock.c      | 12 ++++--
>  dlls/wined3d/utils.c           | 16 ++++----
>  dlls/wined3d/wined3d.spec      |  2 +-
>  dlls/wined3d/wined3d_private.h | 11 +++--
>  include/wine/wined3d.h         |  5 ++-
>  14 files changed, 190 insertions(+), 106 deletions(-)
>
One easy way to split this a little further would be to separate the
d3d11 changes for passing multiple viewports from the wined3d API
changes that allow client libraries to pass multiple viewports.

> -struct wined3d_cs_set_viewport
> +struct wined3d_cs_set_viewports
>  {
>      enum wined3d_cs_op opcode;
> -    struct wined3d_viewport viewport;
> +    struct wined3d_viewport viewports[WINED3D_MAX_VIEWPORTS];
> +    unsigned int viewport_count;
>  };
You can probably save a little CS bandwidth by making the viewports[]
array variable sized. See e.g. wined3d_cs_emit_clear() or
wined3d_cs_mt_push_constants() for examples.

> -static void wined3d_cs_exec_set_viewport(struct wined3d_cs *cs, const void *data)
> +static void wined3d_cs_exec_set_viewports(struct wined3d_cs *cs, const void *data)
>  {
> -    const struct wined3d_cs_set_viewport *op = data;
> +    const struct wined3d_cs_set_viewports *op = data;
>
> -    cs->state.viewport = op->viewport;
> +    if (op->viewport_count)
> +        memcpy(cs->state.viewports, op->viewports, op->viewport_count * sizeof(*op->viewports));
> +    else
> +        memset(cs->state.viewports, 0, sizeof(*cs->state.viewports));
> +    cs->state.viewport_count = op->viewport_count;
>      device_invalidate_state(cs->device, STATE_VIEWPORT);
>  }
>
> -void wined3d_cs_emit_set_viewport(struct wined3d_cs *cs, const struct wined3d_viewport *viewport)
> +void wined3d_cs_emit_set_viewports(struct wined3d_cs *cs, unsigned int viewport_count,
> +        const struct wined3d_viewport *viewports)
>  {
> -    struct wined3d_cs_set_viewport *op;
> +    struct wined3d_cs_set_viewports *op;
>
>      op = cs->ops->require_space(cs, sizeof(*op), WINED3D_CS_QUEUE_DEFAULT);
> -    op->opcode = WINED3D_CS_OP_SET_VIEWPORT;
> -    op->viewport = *viewport;
> +    op->opcode = WINED3D_CS_OP_SET_VIEWPORTS;
> +    if (viewport_count)
> +        memcpy(op->viewports, viewports, viewport_count * sizeof(*viewports));
> +    else
> +        memset(op->viewports, 0, sizeof(*op->viewports));
The memset() here should be unnecessary, since you already handle 0
viewport count in wined3d_cs_exec_set_viewports().

> @@ -811,11 +811,15 @@ void CDECL wined3d_stateblock_capture(struct wined3d_stateblock *stateblock)
>      }
>
>      if (stateblock->changed.viewport
> -            && memcmp(&src_state->viewport, &stateblock->state.viewport, sizeof(stateblock->state.viewport)))
> +            && src_state->viewport_count != stateblock->state.viewport_count
> +            && memcmp(src_state->viewports, stateblock->state.viewports, sizeof(stateblock->state.viewports)))
>      {
Should that second "&&" be a "||"? Should the memcmp() size just be
src_state->viewport_count?

> @@ -5097,7 +5097,7 @@ void get_pointsize(const struct wined3d_context *context, const struct wined3d_s
>
>      if (state->render_states[WINED3D_RS_POINTSCALEENABLE])
>      {
> -        float scale_factor = state->viewport.height * state->viewport.height;
> +        float scale_factor = state->viewports[0].height * state->viewports[0].height;
>
This is ok because WINED3D_RS_POINTSCALEENABLE has no d3d10/11
equivalent, so we can just define it to operate with respect to the
first viewport in wined3d, but that could do with a comment.

> @@ -4106,8 +4109,8 @@ static inline void shader_get_position_fixup(const struct wined3d_context *conte
>
>      position_fixup[0] = 1.0f;
>      position_fixup[1] = 1.0f;
> -    position_fixup[2] = center_offset / state->viewport.width;
> -    position_fixup[3] = -center_offset / state->viewport.height;
> +    position_fixup[2] = center_offset / state->viewports[0].width;
> +    position_fixup[3] = -center_offset / state->viewports[0].height;
>
This is a bit of a problem, since it will potentially result in
incorrect offsets for viewports other than the first. Requiring
ARB_clip_control for multiple viewport support would avoid the issue,
but would disable d3d10/11 support on setups without that extension,
or setups with GL_VIEWPORT_SUBPIXEL_BITS < 8.