[Mesa-dev] [PATCH 2/3] llvmpipe: add support for layered rendering
Jose Fonseca
jfonseca at vmware.com
Fri Jun 7 07:55:57 PDT 2013
----- 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).
> >> + 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