System Environment: -------------------------- Platform: BDW Libdrm: (master)libdrm-2.4.61-4-gfde4969176822fe54197b6baa78f8b0ef900baba Mesa: (master)10aacf5ae8f3e90e2f0967fbdcf96df93e346e20 Xserver: (master)xorg-server-1.17.0-157-gcbb7eb73b5399e31a7afb800363504d539df0ecf Xf86_video_intel:(master)2.99.917-313-gfb1643f0f904eb258da71cd0b8deb8d3ec6dafed Libva: (master)4763db1c2133d4e6b92355938ecb6f23a7767b6b Libva_intel_driver:(master)4a1c4d21f3428b08ef765d7f7de75b97006514ac Kernel: (drm-intel-nightly)f9d865e87168eaba908020942f458c40facd6060 Bug detailed description: ----------------------------- It fails on BDW with Mesa master branch. good commit: 6148e3aae7e0d36b59759075bf7a4ce2248ce321 bad commit: fe74fee8fa721a42448470e063870d24f9453dab output: dEQP Core 2014.x (0xcafebabe) starting.. target implementation = 'X11 EGL/GLX' Test case 'dEQP-GLES3.functional.rasterization.fbo.rbo_multisample_max.primitives.lines_wide'.. Fail (Incorrect rasterization) Test case duration in microseconds = 436136 us DONE! Test run totals: Passed: 0/1 (0.0%) Failed: 1/1 (100.0%) Not supported: 0/1 (0.0%) Warnings: 0/1 (0.0%) Reproduce steps: ---------------------------- 1. xinit 2. ./deqp-gles3 --deqp-gl-context-type=egl --deqp-case="dEQP-GLES3.functional.rasterization.fbo.rbo_multisample_max.primitives.lines_wide"
Bisect shows: fe74fee8fa721a42448470e063870d24f9453dab is the first bad commit. commit fe74fee8fa721a42448470e063870d24f9453dab Author: Iago Toral Quiroga <itoral@igalia.com> AuthorDate: Tue Feb 10 16:40:46 2015 +0100 Commit: Iago Toral Quiroga <itoral@igalia.com> CommitDate: Tue Feb 24 08:58:54 2015 +0100 i965: Fix non-AA wide line rendering with fractional line widths "(...)Let w be the width rounded to the nearest integer (...). If the line segment has endpoints given by (x0,y0) and (x1,y1) in window coordinates, the segment with endpoints (x0,y0-(w-1)/2) and (x1,y1-(w-1/2)) is rasterized, (...)" The hardware it not rounding the line width, so we should do it. Also, we should be careful not to go beyond the hardware limits for the line width after it gets rounded. Gen6-7 define a maximum line width slightly below 8.0, so we should advertise a maximum line width lower than 7.5 to make sure that 7.0 is the maximum integer line width that we can select. Since the line width granularity in these platforms is 0.125, we choose 7.375. Other platforms advertise rounded maximum line widths, so those are fine. Fixes the following 3 dEQP tests: dEQP-GLES3.functional.rasterization.primitives.lines_wide dEQP-GLES3.functional.rasterization.fbo.texture_2d.primitives.lines_wide dEQP-GLES3.functional.rasterization.fbo.rbo_singlesample.primitives.lines_wi Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
From the OpenGL ES 3.1 spec: "13.4.2.1 Wide Lines The actual width of lines is determined by rounding the supplied width to the nearest integer, then clamping it to the implementation-dependent maximum line width." This is what the bad commit implements. In section "13.4.4 Line Multisample Rasterization", the spec talks about multisampled scenarios in particular (which is what the failed test uses), but nothing there seems to contradict the above statement. More over, the test that fails tests multiple line widths, but only fails for the one we advertise as maximum (7.375). If I change that value to 7.250, 7.125 or 7.000, the dEQP test passes just fine. This, as per ther statement in section 13.4.2.1 that I pasted above, is unexpected, since all these line widths are supposed to be rounded to 7.0 and produce the exact same rendering. The fact that dEQP expects a different rendering for 7.375 points to the test being bogus. In fact, not rounding the line-width if multisampling is enabled seems to fix the test, but as I say, that seems to be against the spec, so my conclusion is that this dEQP test is incorrect.
It would be best to get the test fixed. I'm also okay with reducing the line width exposed by the driver a little bit...I don't think anybody really cares.
Created attachment 116303 [details] [review] Option 1: do not round line width with multisampling Fixes the test by not rounding the line width when multisampling is enabled. This seems to be against the spec, but fixes the regressed test. I checked that it does not regress any of the 3 tests fixed with the bad commit.
Created attachment 116306 [details] [review] Option 2: slightly lower maximum line width This fixes the regressed test by advertising a maximum line width of 7.250 instead of the current 7.375, sice for that width the resulting rendering matches dEQP expectations (notice that in the end, since we are rounding the line width to the nearest integer, rendering with with 7.375 and with 7.250 produces the same result, it is just dEQP's expectations that change).
I have attached two patches with two different solutions. My understanding of the specs suggests that the dEQP test is bogus and the bad commit is correct, so I guess we do not want to alter the behavior introduced with that commit and we just want to silence the regressed dEQP test. In that case Option 2) is preferred. Option 1) would be preferred if we think the rules that govern wide-line rendering when multisampling is enabled should be different (although I have not seen anything like that in the spec) or if we see that Option 2) introduces new regressions (I checked that it does not break any of the tests fixed with the bad commit at least). Lu Hua, can you check that the patch in Option 2) does effectively fix the issue in BDW (I have tested in HSW only) and does not introduce new regressions in dEQP?
So we believe that the "offending" patch brings us closer in line with the GLES 3.0 spec, but is breaking DEQP?
(In reply to Gavin Hindman from comment #7) > So we believe that the "offending" patch brings us closer in line with the > GLES 3.0 spec, but is breaking DEQP? Yes, and in fact it fixed three deqp tests.
(In reply to Iago Toral from comment #2) > From the OpenGL ES 3.1 spec: > > "13.4.2.1 Wide Lines > The actual width of lines is determined by rounding the supplied width to > the nearest integer, then clamping it to the implementation-dependent > maximum line width." > > This is what the bad commit implements. In section "13.4.4 Line Multisample > Rasterization", the spec talks about multisampled scenarios in particular > (which is what the failed test uses), but nothing there seems to contradict > the above statement. > > More over, the test that fails tests multiple line widths, but only fails > for the one we advertise as maximum (7.375). If I change that value to > 7.250, 7.125 or 7.000, the dEQP test passes just fine. This, as per ther > statement in section 13.4.2.1 that I pasted above, is unexpected, since all > these line widths are supposed to be rounded to 7.0 and produce the exact > same rendering. The fact that dEQP expects a different rendering for 7.375 > points to the test being bogus. In fact, not rounding the line-width if > multisampling is enabled seems to fix the test, but as I say, that seems to > be against the spec, so my conclusion is that this dEQP test is incorrect. I think you could read the spec either way. Section 13.4.2.1 talks about rendering aliased wide lines, and section 13.4.4 talks about rendering general multisampled lines. Look at similar text in the desktop OpenGL 4.4 spec: The actual width of non-antialiased lines is determined by rounding the supplied width to the nearest integer, then clamping it to the implementation-dependent maximum non-antialiased line width. When ES removed antialiased lines, they removed "non-antialised" but probably should not have. I believe removing the quantization for multisample and antialiased lines is correct.
(In reply to Ian Romanick from comment #9) > (In reply to Iago Toral from comment #2) > > From the OpenGL ES 3.1 spec: > > > > "13.4.2.1 Wide Lines > > The actual width of lines is determined by rounding the supplied width to > > the nearest integer, then clamping it to the implementation-dependent > > maximum line width." > > > > This is what the bad commit implements. In section "13.4.4 Line Multisample > > Rasterization", the spec talks about multisampled scenarios in particular > > (which is what the failed test uses), but nothing there seems to contradict > > the above statement. > > > > More over, the test that fails tests multiple line widths, but only fails > > for the one we advertise as maximum (7.375). If I change that value to > > 7.250, 7.125 or 7.000, the dEQP test passes just fine. This, as per ther > > statement in section 13.4.2.1 that I pasted above, is unexpected, since all > > these line widths are supposed to be rounded to 7.0 and produce the exact > > same rendering. The fact that dEQP expects a different rendering for 7.375 > > points to the test being bogus. In fact, not rounding the line-width if > > multisampling is enabled seems to fix the test, but as I say, that seems to > > be against the spec, so my conclusion is that this dEQP test is incorrect. > > I think you could read the spec either way. Section 13.4.2.1 talks about > rendering aliased wide lines, and section 13.4.4 talks about rendering > general multisampled lines. Look at similar text in the desktop OpenGL 4.4 > spec: > > The actual width of non-antialiased lines is determined by rounding the > supplied width to the nearest integer, then clamping it to the > implementation-dependent maximum non-antialiased line width. > > When ES removed antialiased lines, they removed "non-antialised" but > probably should not have. I believe removing the quantization for > multisample and antialiased lines is correct. Oh, good catch! I just tested this and works without causing any regressions in any of the dEQP wide line rasterization tests. I sent this patch for review: http://lists.freedesktop.org/archives/mesa-dev/2015-June/086031.html
Fixed with: commit f9a18acb56c69b24c1e47cd326dc98e14fadcf94 Author: Iago Toral Quiroga <itoral@igalia.com> Date: Wed Jun 10 09:07:32 2015 +0200 i965: do not round line width when multisampling or antialiaing are enabled In commit fe74fee8fa721a we rounded the line width to the nearest integer to match the GLES3 spec requirements stated in section 13.4.2.1, but that seems to break a dEQP test that renders wide lines in some multisampling scenarios. Ian noted that the Open 4.4 spec has the following similar text: "The actual width of non-antialiased lines is determined by rounding the supplied width to the nearest integer, then clamping it to the implementation-dependent maximum non-antialiased line width." and suggested that when ES removed antialiased lines, they removed "non-antialised" from that paragraph but probably should not have. Going by that note, this patch restricts the quantization implemented in fe74fee8fa721a only to regular aliased lines. This seems to keep the tests fixed with that commit passing while fixing the broken test. v2: - Drop one of the clamps (Ken, Marius) - Add a rule to prevent advertising line widths that when rounded go beyond the limits allowed by the hardware (Ken) - Update comments in the code accordingly (Ian) - Put the code in a utility function (Ian) Fixes: dEQP-GLES3.functional.rasterization.fbo.rbo_multisample_max.primitives.lines_wide Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90749 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> Cc: "10.6" <mesa-stable@lists.freedesktop.org>
Verified and fixed by Mesa 4d35eef326e49cc8da50879d30a1c5088d4775e1 and kernel 4.1.0-rc7_drm-intel-nightly_4643c5_20150613+. Thanks
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.