[Mesa-dev] [PATCH 2/3] llvmpipe: add support for layered rendering
Jose Fonseca
jfonseca at vmware.com
Fri Jun 7 15:16:04 PDT 2013
----- Original Message -----
> Am 07.06.2013 16:55, schrieb Jose Fonseca:
> >
> >
> > ----- Original Message -----
> >> Am 06.06.2013 03:15, schrieb Brian Paul:
> >>> Reviewed-by: Brian Paul <brianp at vmware.com>
> >>>
> >>> Just two minor nits below.
> >>>
> >>>
> >>> On 06/05/2013 05:44 PM, sroland at vmware.com wrote:
> >>>> From: Roland Scheidegger <sroland at vmware.com>
> >>>>
> >>>> Mostly just make sure the layer parameter gets passed through to the
> >>>> right
> >>>> places (and get clamped, can do this at setup time), fix up clears to
> >>>> clear all layers and disable opaque optimization. Luckily don't need to
> >>>> touch the jitted code.
> >>>> (Clears invoked via pipe's clear_render_target method will not work
> >>>> however
> >>>> since the pipe_util_clear function used for it doesn't handle clearing
> >>>> multiple layers yet.)
> >>>> ---
> >>>> src/gallium/drivers/llvmpipe/lp_context.h | 3 +
> >>>> src/gallium/drivers/llvmpipe/lp_jit.h | 2 +-
> >>>> src/gallium/drivers/llvmpipe/lp_rast.c | 195
> >>>> ++++++++++++-----------
> >>>> src/gallium/drivers/llvmpipe/lp_rast.h | 2 +-
> >>>> src/gallium/drivers/llvmpipe/lp_rast_priv.h | 20 ++-
> >>>> src/gallium/drivers/llvmpipe/lp_scene.c | 12 +-
> >>>> src/gallium/drivers/llvmpipe/lp_scene.h | 7 +-
> >>>> src/gallium/drivers/llvmpipe/lp_setup.c | 1 +
> >>>> src/gallium/drivers/llvmpipe/lp_setup_context.h | 1 +
> >>>> src/gallium/drivers/llvmpipe/lp_setup_line.c | 6 +
> >>>> src/gallium/drivers/llvmpipe/lp_setup_point.c | 7 +
> >>>> src/gallium/drivers/llvmpipe/lp_setup_tri.c | 17 +-
> >>>> src/gallium/drivers/llvmpipe/lp_state_derived.c | 13 +-
> >>>> src/gallium/drivers/llvmpipe/lp_texture.c | 3 -
> >>>> src/gallium/drivers/llvmpipe/lp_texture.h | 10 ++
> >>>> 15 files changed, 190 insertions(+), 109 deletions(-)
> >>>>
> >>>> diff --git a/src/gallium/drivers/llvmpipe/lp_context.h
> >>>> b/src/gallium/drivers/llvmpipe/lp_context.h
> >>>> index 54f3830..abfe852 100644
> >>>> --- a/src/gallium/drivers/llvmpipe/lp_context.h
> >>>> +++ b/src/gallium/drivers/llvmpipe/lp_context.h
> >>>> @@ -119,6 +119,9 @@ struct llvmpipe_context {
> >>>> /** Which vertex shader output slot contains viewport index */
> >>>> int viewport_index_slot;
> >>>>
> >>>> + /** Which geometry shader output slot contains layer */
> >>>> + int layer_slot;
> >>>> +
> >>>> /**< minimum resolvable depth value, for polygon offset */
> >>>> double mrd;
> >>>>
> >>>> diff --git a/src/gallium/drivers/llvmpipe/lp_jit.h
> >>>> b/src/gallium/drivers/llvmpipe/lp_jit.h
> >>>> index 4e9ca76..2ecfde7 100644
> >>>> --- a/src/gallium/drivers/llvmpipe/lp_jit.h
> >>>> +++ b/src/gallium/drivers/llvmpipe/lp_jit.h
> >>>> @@ -204,7 +204,7 @@ typedef void
> >>>> const void *dadx,
> >>>> const void *dady,
> >>>> uint8_t **color,
> >>>> - void *depth,
> >>>> + uint8_t *depth,
> >>>> uint32_t mask,
> >>>> struct lp_jit_thread_data *thread_data,
> >>>> unsigned *stride,
> >>>> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c
> >>>> b/src/gallium/drivers/llvmpipe/lp_rast.c
> >>>> index 981dd71..aa5224e 100644
> >>>> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> >>>> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> >>>> @@ -134,6 +134,8 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
> >>>>
> >>>> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> >>>> enum pipe_format format = scene->fb.cbufs[i]->format;
> >>>> + unsigned layer;
> >>>> + uint8_t *map_layer = scene->cbufs[i].map;
> >>>>
> >>>> if (util_format_is_pure_sint(format)) {
> >>>> util_format_write_4i(format, arg.clear_color.i, 0,
> >>>> &uc, 0, 0, 0, 1, 1);
> >>>> @@ -143,14 +145,17 @@ lp_rast_clear_color(struct lp_rasterizer_task
> >>>> *task,
> >>>> util_format_write_4ui(format, arg.clear_color.ui, 0,
> >>>> &uc, 0, 0, 0, 1, 1);
> >>>> }
> >>>>
> >>>> - util_fill_rect(scene->cbufs[i].map,
> >>>> - scene->fb.cbufs[i]->format,
> >>>> - scene->cbufs[i].stride,
> >>>> - task->x,
> >>>> - task->y,
> >>>> - task->width,
> >>>> - task->height,
> >>>> - &uc);
> >>>> + for (layer = 0; layer <= scene->fb_max_layer; layer++) {
> >>>> + util_fill_rect(map_layer,
> >>>> + scene->fb.cbufs[i]->format,
> >>>> + scene->cbufs[i].stride,
> >>>> + task->x,
> >>>> + task->y,
> >>>> + task->width,
> >>>> + task->height,
> >>>> + &uc);
> >>>> + map_layer += scene->cbufs[i].layer_stride;
> >>>> + }
> >>>> }
> >>>> }
> >>>> else {
> >>>
> >>> So, just to be clear (no pun intended), glClear() and pipe->clear()
> >>> should indeed clear all the layers of a multi-layer framebuffer. I
> >>> guess I didn't realize that until I read this code and double-checked
> >>> the spec. Could you put a comment about that here just to be explicit?
> >> Ok. I had to read the specs as well, but it looks like it works like
> >> that indeed for all kind of clears in both d3d10 and opengl (though it
> >> is not very obvious from the d3d10 spec, but it makes sense).
> >> Actually I just checked docs and indeed I've apparently documented this
> >> already in docs/source/context.rst ages ago...
> >>
> >>>
> >>>
> >>>
> >>>> @@ -167,18 +172,21 @@ lp_rast_clear_color(struct lp_rasterizer_task
> >>>> *task,
> >>>> clear_color[3]);
> >>>>
> >>>> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> >>>> -
> >>>> - util_pack_color(arg.clear_color.f,
> >>>> - scene->fb.cbufs[i]->format, &uc);
> >>>> -
> >>>> - util_fill_rect(scene->cbufs[i].map,
> >>>> - scene->fb.cbufs[i]->format,
> >>>> - scene->cbufs[i].stride,
> >>>> - task->x,
> >>>> - task->y,
> >>>> - task->width,
> >>>> - task->height,
> >>>> - &uc);
> >>>> + unsigned layer;
> >>>> + uint8_t *map_layer = scene->cbufs[i].map;
> >>>> + for (layer = 0; layer <= scene->fb_max_layer; layer++) {
> >>>> + util_pack_color(arg.clear_color.f,
> >>>> + scene->fb.cbufs[i]->format, &uc);
> >>>> + util_fill_rect(map_layer,
> >
> > I suspect this pattern will be necessary a lot. Probably makes sense to
> > make a util_fill_volume helper (follow on change maybe).
> I was wondering if it would be useful, though not sure if util_fill_box
> would be better?
Yes, that sounds more consistent.
Jose
I'll do that on a follow-up change as at least the util
> clear render target function could use it too.
>
> Roland
>
> >
> >>>> + scene->fb.cbufs[i]->format,
> >>>> + scene->cbufs[i].stride,
> >>>> + task->x,
> >>>> + task->y,
> >>>> + task->width,
> >>>> + task->height,
> >>>> + &uc);
> >>>> + map_layer += scene->cbufs[i].layer_stride;
> >>>> + }
> >>>> }
> >>>> }
> >>>> }
> >>>
> >>>
> >>>> diff --git a/src/gallium/drivers/llvmpipe/lp_scene.c
> >>>> b/src/gallium/drivers/llvmpipe/lp_scene.c
> >>>> index 771ad08..955942c 100644
> >>>> --- a/src/gallium/drivers/llvmpipe/lp_scene.c
> >>>> +++ b/src/gallium/drivers/llvmpipe/lp_scene.c
> >>>> @@ -151,6 +151,7 @@ lp_scene_begin_rasterization(struct lp_scene *scene)
> >>>> {
> >>>> const struct pipe_framebuffer_state *fb = &scene->fb;
> >>>> int i;
> >>>> + unsigned max_layer = 0xffffff;
> >>>
> >>> unsigned max_layer = ~0UL; ?
> >> Ok, the value doesn't really matter. I actually first wanted to make it
> >> start with implementation layer limit but then noticed it would need to
> >> be the max of 3d and array size so I figured I'll just stick in a large
> >> value instead :-).
> >
> > Maybe a comment on that at least. As 0xffffff really seems arbitrary.
> >
> >>
> >>>
> >>>
> >>>>
> >>>> //LP_DBG(DEBUG_RAST, "%s\n", __FUNCTION__);
> >>>>
> >>>
> >>> -Brian
> >>
> >> Roland
> >
> > Looks good otherwise.
> >
> > Jose
> >
>
More information about the mesa-dev
mailing list