On Wed, Jan 8, 2014 at 7:31 PM, Roland Scheidegger <srol...@vmware.com> wrote: > Am 08.01.2014 19:12, schrieb Jose Fonseca: >> >> >> ----- Original Message ----- >>> Am 08.01.2014 18:27, schrieb jfons...@vmware.com: >>>> From: José Fonseca <jfons...@vmware.com> >>>> >>>> Commit eda21d2a3010d9fc5a68b55a843c5e44b2abf8dd fixed the rasterization >>>> of points for Direct3D but ended up breaking the rasterization of OpenGL >>>> non-sprite points, in particular conform's pntrast.c test. >>>> >>>> The only way to get both working is to properly honour >>>> pipe_rasterizer::point_quad_rasterization, and follow the weird OpenGL >>>> rule when it is false. >>>> --- >>>> src/gallium/drivers/llvmpipe/lp_setup_point.c | 64 >>>> ++++++++++++++++++++++----- >>>> 1 file changed, 54 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_point.c >>>> b/src/gallium/drivers/llvmpipe/lp_setup_point.c >>>> index 988e0c5..e5ce4ee 100644 >>>> --- a/src/gallium/drivers/llvmpipe/lp_setup_point.c >>>> +++ b/src/gallium/drivers/llvmpipe/lp_setup_point.c >>>> @@ -338,9 +338,13 @@ try_setup_point( struct lp_setup_context *setup, >>>> int fixed_width = MAX2(FIXED_ONE, >>>> (subpixel_snap(size) + FIXED_ONE/2 - 1) & >>>> ~(FIXED_ONE-1)); >>>> >>>> - const int x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) - >>>> fixed_width/2; >>>> - const int y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) - >>>> fixed_width/2; >>>> - >>>> + /* Yes this is necessary to accurately calculate bounding boxes >>>> + * with the two fill-conventions we support. GL (normally) ends >>>> + * up needing a bottom-left fill convention, which requires >>>> + * slightly different rounding. >>>> + */ >>>> + int adj = (setup->bottom_edge_rule != 0) ? 1 : 0; >>>> + >>>> struct lp_scene *scene = setup->scene; >>>> struct lp_rast_triangle *point; >>>> unsigned bytes; >>>> @@ -363,13 +367,14 @@ try_setup_point( struct lp_setup_context *setup, >>>> print_point(setup, v0); >>>> >>>> /* Bounding rectangle (in pixels) */ >>>> - { >>>> - /* Yes this is necessary to accurately calculate bounding boxes >>>> - * with the two fill-conventions we support. GL (normally) ends >>>> - * up needing a bottom-left fill convention, which requires >>>> - * slightly different rounding. >>>> + if (!lp_context->rasterizer || >>>> + lp_context->rasterizer->point_quad_rasterization) { >>>> + /* >>>> + * Rasterize points as quads. >>>> */ >>>> - int adj = (setup->bottom_edge_rule != 0) ? 1 : 0; >>>> + >>>> + const int x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) - >>>> fixed_width/2; >>>> + const int y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) - >>>> fixed_width/2; >>>> >>>> bbox.x0 = (x0 + (FIXED_ONE-1)) >> FIXED_ORDER; >>>> bbox.x1 = (x0 + fixed_width + (FIXED_ONE-1)) >> FIXED_ORDER; >>>> @@ -380,8 +385,47 @@ try_setup_point( struct lp_setup_context *setup, >>>> */ >>>> bbox.x1--; >>>> bbox.y1--; >>>> + } else { >>>> + /* >>>> + * OpenGL rasterization rules for non-sprite points. >>> Maybe add "legacy" here before OpenGL? >> >> Sure. >> >>>> + * >>>> + * Per OpenGL 2.1 spec, section 3.3.1, "Basic Point Rasterization". >>>> + */ >>>> + >>>> + const int x0 = subpixel_snap(v0[0][0]); >>>> + const int y0 = subpixel_snap(v0[0][1]) - adj; >>>> + >>>> + int int_width = fixed_width >> FIXED_ORDER; >>>> + >>>> + assert(setup->pixel_offset != 0); >>>> + >>>> + if (int_width == 1) { >>>> + bbox.x0 = x0 >> FIXED_ORDER; >>>> + bbox.y0 = y0 >> FIXED_ORDER; >>>> + bbox.x1 = bbox.x0; >>>> + bbox.y1 = bbox.y0; >>>> + } else { >>>> + if (int_width & 1) { >>>> + /* Odd width */ >>>> + bbox.x0 = (x0 >> FIXED_ORDER) - (int_width - 1)/2; >>>> + bbox.y0 = (y0 >> FIXED_ORDER) - (int_width - 1)/2; >>>> + } else { >>>> + /* Even width */ >>>> + bbox.x0 = ((x0 + FIXED_ONE/2) >> FIXED_ORDER) - int_width/2; >>>> + bbox.y0 = ((y0 + FIXED_ONE/2) >> FIXED_ORDER) - int_width/2; >>>> + } >>>> + >>>> + bbox.x1 = bbox.x0 + int_width - 1; >>>> + bbox.y1 = bbox.y0 + int_width - 1; >>>> + } >>>> } >>>> - >>>> + >>>> + if (0) { >>>> + debug_printf(" bbox: (%i, %i) - (%i, %i)\n", >>>> + bbox.x0, bbox.y0, >>>> + bbox.x1, bbox.y1); >>>> + } >>>> + >>>> if (!u_rect_test_intersection(&setup->draw_regions[viewport_index], >>>> &bbox)) { >>>> if (0) debug_printf("offscreen\n"); >>>> LP_COUNT(nr_culled_tris); >>>> >>> >>> This looks good to me. Note though this type of point rasterization is >>> only available in pre 3.0 contexts (or compatibilility contexts which we >>> don't support) anyway. >> >> Oh really!? Argh.. It though it was still relevant nowadays, or I wouldn't >> have wasted my afternoon on trying to figure out the regression and the >> proper fix... > Well it seems at least some hw (nvidia for one) still can follow these > rules, so maybe there are apps out there which rely on it (radeonsi otoh > ignores the state). At least conform relies on it apparently so it's not > really a waste :-).
I have never understood what point_quad_rasterization means and how we should use it. I think Radeons always rasterize points as axis-aligned rectangles (width and height can be different) and there is no support for round/smooth points as far as I know. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev