I can't remember why I memset them to 0x88. I don't think it's
necessary, so I'll remove it.
I think I will memset the pipe_sample_locations_state to zero though
so cso_set_sample_locations() works better.
On Mon, May 7, 2018 at 4:30 PM, Brian Paul <bri...@vmware.com> wrote:
> More nit-picks below.
>
>
>
> On 05/04/2018 06:09 AM, Rhys Perry wrote:
>>
>> Signed-off-by: Rhys Perry <pendingchao...@gmail.com>
>> ---
>> src/mesa/state_tracker/st_atom_framebuffer.c | 64
>> ++++++++++++++++++++++++++++
>> src/mesa/state_tracker/st_cb_msaa.c | 22 ++++++++++
>> src/mesa/state_tracker/st_extensions.c | 1 +
>> 3 files changed, 87 insertions(+)
>>
>> diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c
>> b/src/mesa/state_tracker/st_atom_framebuffer.c
>> index 3ef3ff34a9..bb5f02125f 100644
>> --- a/src/mesa/state_tracker/st_atom_framebuffer.c
>> +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
>> @@ -102,6 +102,68 @@ framebuffer_quantize_num_samples(struct st_context
>> *st, unsigned num_samples)
>> return quantized_samples;
>> }
>> +/**
>> + * Update the pipe_context's sample location state
>> + */
>> +static void
>> +update_sample_locations(struct st_context *st,
>> + const struct pipe_framebuffer_state *fb_state)
>> +{
>> + struct pipe_sample_locations_state locations;
>> + struct gl_framebuffer *fb = st->ctx->DrawBuffer;
>> +
>> + if (!st->ctx->Extensions.ARB_sample_locations)
>> + return;
>> +
>> + locations.enabled = fb->ProgrammableSampleLocations;
>> + if (locations.enabled) {
>> + unsigned grid_width, grid_height;
>> + int samples = _mesa_geometric_samples(fb);
>> + int pixel, sample_index;
>> + bool sample_location_pixel_grid = fb->SampleLocationPixelGrid;
>> +
>> + st->pipe->get_sample_pixel_grid(st->pipe, samples, &grid_width,
>> &grid_height);
>> +
>> + /**
>> + * when a dimension is greater than MAX_SAMPLE_LOCATION_GRID_SIZE,
>> + * st->ctx->Driver.GetSamplePixelGrid() returns 1 for both
>> dimensions.
>> + */
>> + if (grid_width>MAX_SAMPLE_LOCATION_GRID_SIZE ||
>> + grid_height>MAX_SAMPLE_LOCATION_GRID_SIZE)
>
>
> Space before/after >
>
>
>> + sample_location_pixel_grid = false;
>> +
>> + for (pixel = 0; pixel < grid_width * grid_height; pixel++) {
>> + for (sample_index = 0; sample_index < samples; sample_index++) {
>> + int table_index = sample_index;
>> + float x = 0.5f, y = 0.5f;
>> + uint8_t loc;
>> + if (sample_location_pixel_grid)
>> + table_index = pixel * samples + sample_index;
>> + if (fb->SampleLocationTable) {
>> + x = fb->SampleLocationTable[table_index*2];
>> + y = fb->SampleLocationTable[table_index*2+1];
>> + }
>> + if (st->state.fb_orientation == Y_0_BOTTOM)
>> + y = 1.0 - y;
>> +
>> + loc = roundf(CLAMP(x*16.0f, 0.0f, 15.0f));
>> + loc |= (int)roundf(CLAMP(y*16.0f, 0.0f, 15.0f)) << 4;
>
>
> spaces before/after *
>
>
>> + locations.locations[pixel*samples+sample_index] = loc;
>
>
> Spaces before/after *, +
>
>
>> + }
>> + }
>> +
>> + util_sample_locations_flip_y(st->pipe, &locations, fb_state);
>> + } else {
>> + /**
>> + * util_sample_locations_flip_y() initializes unused data to 0x88,
>> so
>> + * this memset is not useful when locations.enabled is true.
>> + */
>> + memset(locations.locations, 0x88, sizeof(locations.locations));
>
>
> OK, what's the significance of 0x88 here and in the previous patch?
>
>
>
>> + }
>> +
>> + cso_set_sample_locations(st->cso_context, &locations);
>> +}
>> +
>> /**
>> * Update framebuffer state (color, depth, stencil, etc. buffers)
>> */
>> @@ -209,4 +271,6 @@ st_update_framebuffer_state( struct st_context *st )
>> st->state.fb_num_samples =
>> util_framebuffer_get_num_samples(&framebuffer);
>> st->state.fb_num_layers =
>> util_framebuffer_get_num_layers(&framebuffer);
>> st->state.fb_num_cb = framebuffer.nr_cbufs;
>> +
>> + update_sample_locations(st, &framebuffer);
>> }
>> diff --git a/src/mesa/state_tracker/st_cb_msaa.c
>> b/src/mesa/state_tracker/st_cb_msaa.c
>> index 7f1b4fde91..092e74d28e 100644
>> --- a/src/mesa/state_tracker/st_cb_msaa.c
>> +++ b/src/mesa/state_tracker/st_cb_msaa.c
>> @@ -56,8 +56,30 @@ st_GetSamplePosition(struct gl_context *ctx,
>> }
>> +static void
>> +st_GetProgrammableSampleCaps(struct gl_context *ctx, struct
>> gl_framebuffer *fb,
>> + GLuint *outBits, GLuint *outWidth, GLuint
>> *outHeight)
>> +{
>> + struct st_context *st = st_context(ctx);
>> +
>> + st_validate_state(st, ST_PIPELINE_UPDATE_FRAMEBUFFER);
>> +
>> + if (st->pipe->get_sample_pixel_grid)
>> + st->pipe->get_sample_pixel_grid(st->pipe,
>> _mesa_geometric_samples(fb),
>> + outWidth, outHeight);
>> + *outBits = 4;
>> +
>> + /* We could handle this better in some circumstances,
>> + * but it's not really an issue */
>> + if (*outWidth>MAX_SAMPLE_LOCATION_GRID_SIZE ||
>> *outHeight>MAX_SAMPLE_LOCATION_GRID_SIZE) {
>
>
> spaces before/after > It that line longer than 78 columns?
>
>
>
>> + *outWidth = 1;
>> + *outHeight = 1;
>> + }
>> +}
>> +
>> void
>> st_init_msaa_functions(struct dd_function_table *functions)
>> {
>> functions->GetSamplePosition = st_GetSamplePosition;
>> + functions->GetProgrammableSampleCaps = st_GetProgrammableSampleCaps;
>> }
>> diff --git a/src/mesa/state_tracker/st_extensions.c
>> b/src/mesa/state_tracker/st_extensions.c
>> index 0dc8adb262..b74fb63cef 100644
>> --- a/src/mesa/state_tracker/st_extensions.c
>> +++ b/src/mesa/state_tracker/st_extensions.c
>> @@ -636,6 +636,7 @@ void st_init_extensions(struct pipe_screen *screen,
>> { o(ARB_query_buffer_object),
>> PIPE_CAP_QUERY_BUFFER_OBJECT },
>> { o(ARB_robust_buffer_access_behavior),
>> PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR },
>> { o(ARB_sample_shading), PIPE_CAP_SAMPLE_SHADING
>> },
>> + { o(ARB_sample_locations),
>> PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS },
>> { o(ARB_seamless_cube_map), PIPE_CAP_SEAMLESS_CUBE_MAP
>> },
>> { o(ARB_shader_ballot), PIPE_CAP_TGSI_BALLOT
>> },
>> { o(ARB_shader_clock), PIPE_CAP_TGSI_CLOCK
>> },
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
