Bug 34495 - Selecting objects in Blender 2.56 slow due the software gl_select mode
Summary: Selecting objects in Blender 2.56 slow due the software gl_select mode
Status: RESOLVED WONTFIX
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: 9.1
Hardware: x86-64 (AMD64) Linux (All)
: highest critical
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
: 38422 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-02-19 23:13 UTC by Lars G
Modified: 2018-04-12 12:45 UTC (History)
9 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Xorg Logfile (14.96 KB, application/octet-stream)
2011-02-19 23:13 UTC, Lars G
Details
fix llvm_pipeline_generic (1.74 KB, patch)
2011-06-27 03:09 UTC, Micael Dias
Details | Splinter Review
experimental hw support for GL_SELECT (12.67 KB, patch)
2011-06-30 10:59 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
hw support for GL_SELECT v2 (15.12 KB, patch)
2011-07-01 03:02 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
crash fix (3.66 KB, patch)
2011-07-02 00:35 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
GL_SELECT hw support v3 (18.18 KB, patch)
2011-07-03 06:23 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
workaround for issue in comment #38 #39 (4.70 KB, patch)
2011-07-04 14:08 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
GL_SELECT hw support v4 (16.64 KB, patch)
2011-07-11 14:58 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
GL_SELECT hw support v5 (16.96 KB, patch)
2011-07-17 11:45 UTC, Pierre-Eric Pelloux-Prayer
Details | Splinter Review
Implement HW accelerated GL_SELECT (18.97 KB, patch)
2011-08-01 20:44 UTC, Micael Dias
Details | Splinter Review
Arch Linux PKGBUILD (2.45 KB, text/plain)
2013-08-15 17:59 UTC, Lars G
Details

Description Lars G 2011-02-19 23:13:29 UTC
Created attachment 43567 [details]
Xorg Logfile

In Blender 2.56 selecting objects is verry slow.
Maybe this has someting todo with the "r600: fix blender picking" patch from
http://cgit.freedesktop.org/mesa/mesa/commit/?id=cf8af9bcf127e170b64112bd548d5d4e79c8e894
not applied to the gallium driver?

This is with:
mesa-dri-drivers-7.10-0.26.fc15.x86_64
on an HP Compaq 6830s
with ATI Mobility Radeon HD 3430.


Thanks Lars
Comment 1 Lars G 2011-06-16 09:05:02 UTC
Still the same issue here with fully updated Fedora 15.
This is *really* annoying, because it makes working in Blender's "object mode" nearly impossible.
Comment 2 Lars G 2011-06-16 09:11:58 UTC
Forgot to say that it's not only really slow, but it also *crashes* nearly everytime when selecting something in this mode. (via viewport, selecting from the outliner list seems ok)

Thanks,
Lars
Comment 3 Sven Arvidsson 2011-06-16 11:51:06 UTC
I just tried this with Blender 2.57 and current git master on my HD5670 and I don't experience any problems.

So it could be that this bug is specific to r600, or (hopefully) that it's fixed in git master.
Comment 4 Lars G 2011-06-16 15:38:49 UTC
(In reply to comment #3)
> I just tried this with Blender 2.57 and current git master on my HD5670 and I
> don't experience any problems.

Thanks for testing Sven!

Can you try the following in Blender:
- Add a multiresolution modifier (with at least 7 subdivides) to the default cube to get a lot more vertices.
- In object mode, select/deselect via viewport clicking several times.
This crashes here *verry* often.

Thanks,
Lars

ps. In Blender preferences>system i use the triple buffer window draw method.
With the other settings the gui slows down a lot.
Comment 5 Pierre-Eric Pelloux-Prayer 2011-06-17 02:05:52 UTC
I can reproduce the crash as well.
Some additional notes :
- I'm using : HD4850, blender 2.57.2, mesa fc8c4a3a7b92a1134cd3a9312063abba9e14b0fe
- with LIBGL_ALWAYS_SOFTWARE=1 => no crash
- here's the segfault backtrace :
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7eacf1c in draw_llvm_shader1 ()
(gdb) whe
#0  0x00007ffff7eacf1c in draw_llvm_shader1 ()
#1  0x00007fffe88f0c8b in llvm_pipeline_generic (middle=0x1f75ea0, fetch_info=0x7ffffffd5df0, prim_info=0x7ffffffd5dc0) at draw/draw_pt_fetch_shade_pipeline_llvm.c:246
#2  0x00007fffe88f0f51 in llvm_middle_end_linear_run (middle=0x1f75ea0, start=4096, count=190, prim_flags=0) at draw/draw_pt_fetch_shade_pipeline_llvm.c:364
#3  0x00007fffe8884d9b in vsplit_segment_simple_linear (vsplit=0x1f73050, flags=0, istart=4096, icount=190) at draw/draw_pt_vsplit_tmp.h:237
#4  0x00007fffe88850a0 in vsplit_run_linear (frontend=0x1f73050, start=4096, count=190) at draw/draw_split_tmp.h:61
#5  0x00007fffe887d246 in draw_pt_arrays (draw=0x1f145f0, prim=3, start=4096, count=190) at draw/draw_pt.c:113
#6  0x00007fffe887de5b in draw_vbo (draw=0x1f145f0, info=0x7ffffffd5f80) at draw/draw_pt.c:491
#7  0x00007fffe887dca6 in draw_arrays_instanced (draw=0x1f145f0, mode=3, start=4096, count=190, startInstance=0, instanceCount=1) at draw/draw_pt.c:408
#8  0x00007fffe887dc1a in draw_arrays (draw=0x1f145f0, prim=3, start=4096, count=190) at draw/draw_pt.c:376
#9  0x00007fffe8844a0d in st_feedback_draw_vbo (ctx=0x1eae500, arrays=0x1f007f0, prims=0x1efee0c, nr_prims=9, ib=0x0, index_bounds_valid=1 '\001', min_index=0, max_index=4285) at state_tracker/st_draw_feedba
ck.c:262
Comment 6 Sven Arvidsson 2011-06-17 04:28:37 UTC
Still can't reproduce it here, even with the instructions Lars posted.

I wonder if the version of llvm have an impact (since the crash happens in draw_llvm_shader1) I'm using version 2.9.
Comment 7 Lars G 2011-06-17 12:50:42 UTC
(In reply to comment #5)

thanks for testing Pierre-Eric!

> I can reproduce the crash as well.
> - with LIBGL_ALWAYS_SOFTWARE=1 => no crash
Same here.

after installing all needed debuginfos, running in gdb i get:
...
Reading symbols from /usr/bin/blender...Reading symbols from /usr/lib/debug/usr/bin/blender.debug...done.
...
Starting program: /usr/bin/blender 
[Thread debugging using libthread_db enabled]
warning: "/usr/lib/debug/usr/lib64/dri/libllvmcore-2.8.so.debug": separate debug info file has no debug info
...
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff4e40814 in ?? ()
(gdb) whe
#0  0x00007ffff4e40814 in ?? ()
#1  0x0000000000000dd0 in ?? ()
#2  0x0000f83400000007 in ?? ()
#3  0x000007be00000800 in ?? ()
#4  0x0000000001f12ac0 in ?? ()
#5  0x0000000000000007 in ?? ()
#6  0x00007ffffffd5e40 in ?? ()
#7  0x0000000000000000 in ?? ()

so i think there is another bug in fedora's mesa-debuginfo package not providing the right info in
libllvmcore-2.8.so.debug
(i filed this mesa-debuginfo bug in the redhat bugzilla.)

as i get no debug info from gdb and only the libllvmcore-2.8.so debuginfo is missing, it looks like the same area where this bug comes from to me.


(In reply to comment #6)

> I wonder if the version of llvm have an impact (since the crash happens in draw_llvm_shader1) I'm using version 2.9.

i don't have a separate llvm package installed on my system.
looks like the crash then comes from:
http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c#n246

but i'm not good in coding such things...
but i can help testing/debugging this.

thanks,
lars
Comment 8 Alex Deucher 2011-06-17 13:51:04 UTC
*** Bug 38422 has been marked as a duplicate of this bug. ***
Comment 9 Pierre-Eric Pelloux-Prayer 2011-06-18 10:12:06 UTC
I just upgraded from llvm 2.8.1 to llvm 2.9

I do not have crash anymore, BUT once the multiresolution modifier has 7+ subdivision, Blender is extremely slow.
Well not all Blender, at least the selection process is.
It goes like this :
- when doing nothing blender use <5% cpu
- right click in the scene => cpu goes to 100% for 10 seconds
- then cpu usage goes back to normal

Using gdb, I paused randomly Blender execution when cpu was high, and it always gave me this stacktrace :
[...]
#4  0x00007fffe921ee6e in tgsi_exec_machine_run (mach=0x1f10fa0) at tgsi/tgsi_exec.c:4036
#5  0x00007fffe9209128 in vs_exec_run_linear (shader=0x2d1c3e0, input=0x3478634, output=0x34c0fc4, constants=0x1f07248, const_size=0x1f07348, count=2728, input_stride=84, output_stride=84) at draw/draw_vs_ex
ec.c:149
#6  0x00007fffe9200abb in draw_vertex_shader_run (vshader=0x2d1c3e0, constants=0x1f07248, const_size=0x1f07348, input_verts=0x7ffffffd5d00, output_verts=0x7ffffffd5ce0) at draw/draw_pt_fetch_shade_pipeline.c
:189
#7  0x00007fffe9200c25 in fetch_pipeline_generic (middle=0x1f10d70, fetch_info=0x0, prim_info=0x7ffffffd5dc0) at draw/draw_pt_fetch_shade_pipeline.c:238
#8  0x00007fffe9200e6a in fetch_pipeline_linear_run (middle=0x1f10d70, start=0, count=2728, prim_flags=0) at draw/draw_pt_fetch_shade_pipeline.c:345
#9  0x00007fffe9205e77 in vsplit_segment_simple_linear (vsplit=0x1f0e150, flags=0, istart=0, icount=2728) at draw/draw_pt_vsplit_tmp.h:237
#10 0x00007fffe920617c in vsplit_run_linear (frontend=0x1f0e150, start=0, count=2728) at draw/draw_split_tmp.h:61
#11 0x00007fffe91fe34e in draw_pt_arrays (draw=0x1f06c40, prim=7, start=0, count=2728) at draw/draw_pt.c:113
#12 0x00007fffe91fef39 in draw_vbo
Comment 10 Micael Dias 2011-06-18 20:50:24 UTC
Still crashes here.

$ llvm-config --version
3.0svn
Comment 11 Lars G 2011-06-19 17:18:50 UTC
(In reply to comment #10)
> Still crashes here.

Same crashing here with latest git and llvm 2.9.
Comment 12 Micael Dias 2011-06-24 00:33:00 UTC
For the record, blender doesn't seem to crash when enabling it's "VBO rendering mode" for viewports. However this makes blender's interface very slow.

When disabling the VBO mode, it starts crashing at a "movss" instruction inside "draw_llvm_shader()".
Comment 13 Micael Dias 2011-06-25 22:18:46 UTC
I've been doing some debugging on this and found something odd.

I enabled the lp_build_printf call on auxiliary/draw/draw_llvm.c, function generate_fetch() and noticed that when blender crashes, the output is ALWAYS this:

...
vbuf index = 0, stride is 65484
vbuf index = 0, stride is 65496
vbuf index = 0, stride is 65508
vbuf index = 0, stride is 65520
vbuf index = 0, stride is 65532

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ecf24e in draw_llvm_shader1 ()


It always crashes when stride is 65532 regardless of the scene I am using in blender to try and crash it.
I should add that when it doesn't crash, there are times when stride goes a little bit above that, but not much.

I'm not sure it helps in debugging the problem, but maybe it does...
Comment 14 Micael Dias 2011-06-27 03:09:02 UTC
Created attachment 48457 [details] [review]
fix llvm_pipeline_generic
Comment 15 Micael Dias 2011-06-27 03:10:09 UTC
I just uploaded the "fix llvm_pipeline_generic" attachment which is a patch that seems to fix it.

Can anyone please test?
Comment 16 Sergei T 2011-06-27 06:23:44 UTC
patch fix problem here (Fedora 15, blender/cycles from svn, mesa trunk+patch, x86_64, radeon 5670)

Many thanks, I almost gave up to find solution several last months. (for some reason crash stack was too scrambled with IP looks more like float point number then real address )
Comment 17 Pierre-Eric Pelloux-Prayer 2011-06-27 07:38:42 UTC
The massive slowdown observed when selecting an object seems to come from Blender's usage of GL_SELECT rendering mode, which AFAIK is software-rendering.

Hardware support for GL_SELECT in Mesa should solve the problem, but I'm not sure this is going to happen.
Comment 18 Micael Dias 2011-06-28 06:53:45 UTC
@Sergei: no problem. This bug was also hitting on my nerves for months, and I was always curious about how the linux graphics stack actually works, so I decided to see if I could see where this bug was comming from.
After having llvm generate some printfs it was pretty obvious that the issue was some kind of buffer overflow. Then generating hardcoded values on generate_fetch() and not having the crash anymore was a key clue that that was corrupting the memory somehow. Funny enough draw_llvm_generate_elts() checks for these buffer overflows, so it was almost copy-paste from there. I'm not even sure this is a proper fix since draw_llvm_generate_elts does a little more than I do here.

@Pierre: It's a pitty if it doesn get hardware accelerated. Many CAD apps use GL_SELECT. Can you please give me a hint on how one would go about implementing HW acceleration for it? I'm not sure I'm able to do it, but would be interested in playing with it.
Comment 19 Pierre-Eric Pelloux-Prayer 2011-06-30 10:58:55 UTC
> @Pierre: It's a pitty if it doesn get hardware accelerated. Many CAD apps use
> GL_SELECT. Can you please give me a hint on how one would go about implementing
> HW acceleration for it? I'm not sure I'm able to do it, but would be interested
> in playing with it.

I've been toying with this idea for a few hours, and the next attachement will be my current patch bringing sort of HW acceleration for GL_SELECT. 
(it should work for any Gallium driver, not only r600g)
** WARNING ** it's experimental AND rough AND buggy but it works well for the specific test case above (Blender + 1 cube + multiresolution modifier level 8) : it reduces selection time from +30 secs to ~1sec.

I'm posting it here even if it's not finished, to get some comments on how it's done and if Mesa/gallium developpers think it's worth the effort.

(please note that left click on 3D view in Blender will crash...)

It's done using :
- an offscreen surface where objects are drawn when in GL_SELECT mode
- a custom pixel shader which outputs object's name + depth in color buffer
- glQueries
- glReadBack of offscreen surface when glQuery is >0 to read back name & z values to feed the selection buffer
Comment 20 Pierre-Eric Pelloux-Prayer 2011-06-30 10:59:39 UTC
Created attachment 48617 [details] [review]
experimental hw support for GL_SELECT

(tested with r600g and nouveau)
Comment 21 Pierre-Eric Pelloux-Prayer 2011-07-01 03:02:40 UTC
Created attachment 48646 [details] [review]
hw support for GL_SELECT v2

Here's a new version of the patch :
- I've simplified it a bit (removed the fragment program)
- corrected a few bugs (no more crash=
- tried to improve performance by batching query/readback

It only works with Gallium-based drivers (and breaks GL_SELECT on non-Gallium drivers), but that should be easy to fix.

I'd appreciate some testing by other users to confirm that it's working (and is faster than the current implementation).
Comment 22 Micael Dias 2011-07-01 03:42:54 UTC
Heh, I just tested your 1st version now and you just released the second :)

The 1st version was working nice except when using a very high level of subdivisions (tested with luxball3 http://www.luxrender.net/forum/viewtopic.php?f=13&t=1383) with the subsurf modifier, which would crash with "Query result: 2147483647".
As far as performance goes, now when using VBOs and picking an object it seems just as fast as previously without using VBOs, which is a great improvement.

I will test the 2nd version as soon as I can and provide some feedback.
Thank you for this work :)
Comment 23 Pierre-Eric Pelloux-Prayer 2011-07-01 04:02:57 UTC
(In reply to comment #22)
> Heh, I just tested your 1st version now and you just released the second :)
> 
> The 1st version was working nice except when using a very high level of
> subdivisions (tested with luxball3
> http://www.luxrender.net/forum/viewtopic.php?f=13&t=1383) with the subsurf
> modifier, which would crash with "Query result: 2147483647".

I've just made the same test and it crashed too.
The problem is the driver falls in an infinite loop :
r600_context_flush -> r600_context_queries_resume -> r600_query_begin -> r600_context_flush -> ...
If you're using r600g driver too, maybe that's a hint there's a bug in how it handles queries/flushes (or it's simply a bug in my patch :-))
I'll have a look at this specific problem.
Comment 24 Micael Dias 2011-07-01 04:06:25 UTC
Yes, I am using r600g.

Maybe you can fix it in your code to prevent such large loops, but there seems to be a bug in the driver itself since it crashes the GPU instead of failing gracefully.
Comment 25 Lars G 2011-07-01 04:22:58 UTC
Great work!

Applied both patches (llvm fix and gl_select v2)
and it works nice and speedy for me in a shaded view!! *woohoo* :)

When i select a hires mesh in wireframe mode it crashes, a shaded view seems ok.
Will do further tests asap.

Thanks!!
Comment 26 Micael Dias 2011-07-01 08:10:16 UTC
@Pierre: After looking at your patch for a while I have the following doubts/suggestions:

1) on _mesa_exec_select_batch() why would you loop through buffer[..] for each batch entry? at first sight it seems to me that if there is more than one entry, the minZ and maxZ values written by write_hit_record would be the same for every entry.

2) What happens if the user changes the fbo or any state needed (depth and scissor) after calling "glRenderMode(GL_SELECT)"? Maybe we should create our st_select_draw_vbo that makes sure to have the correct state->render using st_draw_vbo->restore state. It is possible that this could be a little heavy on state changes but it feels safer... Maybe we could implement some kind of "state-override" later to make sure we only restore states after exiting GL_SELECT mode and still be sure that the end user isn't interfering with our GL_SELECT implementation.
Comment 27 Pierre-Eric Pelloux-Prayer 2011-07-01 09:00:56 UTC
(In reply to comment #26)
> @Pierre: After looking at your patch for a while I have the following
> doubts/suggestions:
> 
> 1) on _mesa_exec_select_batch() why would you loop through buffer[..] for each
> batch entry? at first sight it seems to me that if there is more than one
> entry, the minZ and maxZ values written by write_hit_record would be the same
> for every entry.

This is what I understood from GL_SELECT behavior : "Both the minimum and maximum window-coordinate z values of all vertices of the primitives that intersected the viewing volume"
So by looping over buffer[] I'm trying to find the min/max z for the entry (at least that was the point, but maybe it's incorrectly done :-))

> 
> 2) What happens if the user changes the fbo or any state needed (depth and
> scissor) after calling "glRenderMode(GL_SELECT)"? 

What happens if user changes something : it will not work reliably... Thanks to remind me of this problem which I almost forgot.
Now the necessary state change should be limited :
- GL_DEPTH_TEST : I don't understand why it doesn't work with it enabled, but it definitely not be changed
- GL_SCISSOR : this one is more annoying, as it's wrong to disable it too (maybe the user really wants to GL_SELECT on a sub area of the window). But as glScissor take window coordinates values, those should be changed before drawing (scissor_width would become : scissor_width * fbo_width / window_width)
- FBO is needed

> Maybe we should create our
> st_select_draw_vbo that makes sure to have the correct state->render using
> st_draw_vbo->restore state. It is possible that this could be a little heavy on
> state changes but it feels safer... Maybe we could implement some kind of
> "state-override" later to make sure we only restore states after exiting
> GL_SELECT mode and still be sure that the end user isn't interfering with our
> GL_SELECT implementation.

The 1st version of the patch had a custom drawing function ('select_draw_func'). Initially I was doing some state changes over there, but it caused problem in 'vbo_exec_FlushVertices' with this assert : assert(exec->flush_call_depth == 1);
But sure, it would be safer to have some state check/change over there.
Comment 28 Micael Dias 2011-07-01 10:07:58 UTC
(In reply to comment #27)
> This is what I understood from GL_SELECT behavior : "Both the minimum and
> maximum window-coordinate z values of all vertices of the primitives that
> intersected the viewing volume"
> So by looping over buffer[] I'm trying to find the min/max z for the entry (at
> least that was the point, but maybe it's incorrectly done :-))

Yes but you're reading exacly the same buffer for each entry, therefore returning the minZ and maxZ values of all the entries combined. That was my interpretation of the code, but maybe I missed something.


> What happens if user changes something : it will not work reliably... Thanks to
> remind me of this problem which I almost forgot.
> Now the necessary state change should be limited :
> - GL_DEPTH_TEST : I don't understand why it doesn't work with it enabled, but
> it definitely not be changed

This got me thinking, and I think I know why. I believe currently minZ and maxZ values returned will almost always be incorrect. This is because at the end of a draw call without depth test you never know if the outputted pixels have the minimum or maximum Z values. I think that in order to correctly do this we would have to have 2 fbos. And we would render like this:

- enable depth test
- enable depth write
- select 1st fbo
--- set depth test function to "greater than"
--- render
- select 2nd fbo
--- set depth test function to "lesser than"
--- render
- loop through both fbos' buffers to determine minZ and maxZ

Unfortunately that last step will probably kill performance because we will have to wait for the GPU, but there's no other way we can know which Z values belong to which entries (unless we would allocate different pairs of FBOs for each entry, which is obvisually unviable).
I still believe this would still be a lot faster than a software implementation, specially with high poly meshes, which is where it matters the most actually.
Comment 29 Micael Dias 2011-07-01 10:20:43 UTC
More food for thought:

Using an fbo which is bigger than 1x1 is a waste of space since we only want to know Z values. Using a 32x32 fbo with some custom shaders we could handle 32*32 entries and so on.

With some more shader magic maybe we could have the left half of the fbo for minZ values and the other half for maxZ values?

Am I dreaming or is this doable?
Comment 30 Pierre-Eric Pelloux-Prayer 2011-07-01 10:28:11 UTC
(In reply to comment #28)
> Yes but you're reading exacly the same buffer for each entry

The FBO is read once, but it contains ctx->Select.BatchEntryCount entries (each entry is a square of size : _OffscreenSurfaceWidth * _OffscreenSurfaceHeight, the total FBO width is _OffscreenSurfaceWidth * BatchSize)

By the way the glReadBack could be changed from :
_mesa_ReadPixels(0, 0, fboW, fboH, GL_DEPTH_COMPONENT, GL_FLOAT, buffer);
to :
_mesa_ReadPixels(0, 0, _OffscreenSurfaceWidth * ctx->Select.BatchEntryCount, fboH, GL_DEPTH_COMPONENT, GL_FLOAT, buffer);


In the loop, we're reading this value :
float z = buffer[y * fboW + xOffset + x];

with xOffset being modified according to the current entry being read (ie point at the correct square)
So I keep thinking this is correct :-)


> This got me thinking, and I think I know why. I believe currently minZ and maxZ
> values returned will almost always be incorrect. 

Yup, you're right...

> [...] I think that in order to correctly do this we
> would have to have 2 fbos.

Hmm... this would further complicate the process but maybe that's the only solution. I have to think about it too :-)
Comment 31 Pierre-Eric Pelloux-Prayer 2011-07-01 10:33:20 UTC
(In reply to comment #29)
> More food for thought:
> 
> Using an fbo which is bigger than 1x1 is a waste of space since we only want to
> know Z values. Using a 32x32 fbo with some custom shaders we could handle 32*32
> entries and so on.
> 
> With some more shader magic maybe we could have the left half of the fbo for
> minZ values and the other half for maxZ values?
> 
> Am I dreaming or is this doable?


I tried smaller FBO (8x8 I think), but the selection in Blender worked less reliably. But maybe that was related to wrong Z values being returned, and thus causing problem to Blender's selection algorithm.
And, using 2 FBO might not be that expensive 1) if they are small 2) if state changes and read back are batched
But I'm far from being an expert, so I guess we'll need to build it first to see if how well it performs.

Thanks for the help !
Comment 32 Pierre-Eric Pelloux-Prayer 2011-07-02 00:35:58 UTC
Created attachment 48680 [details] [review]
crash fix

Here's a patch fixing (hopefully) the crash reported in comment #22

It add some logic to r600g query handling, to avoid unnecessary flushes and infinite loop (though the crash is less easy to reproduce with git master because commit 61c976c3ccc815aaf0128eea835aee9667cd8bbe reduces flush count)
Comment 33 Lars G 2011-07-02 05:16:55 UTC
(In reply to comment #32)
> Here's a patch fixing (hopefully) the crash reported in comment #22

This fixes the wireframe-selection crasher for me.
Looks like with all three patches applied all crashing/slowdowns are now cured here!
Will put it on the test with some hires blender modeling now.

Again, *thanks*! :)
Comment 34 Micael Dias 2011-07-02 08:52:05 UTC
The fix llvm patch is no longer necessary, it's already commited to master.
Comment 35 Micael Dias 2011-07-02 10:32:48 UTC
(In reply to comment #32)
> Created an attachment (id=48680) [details]
> crash fix
> 
> Here's a patch fixing (hopefully) the crash reported in comment #22
> 
> It add some logic to r600g query handling, to avoid unnecessary flushes and
> infinite loop (though the crash is less easy to reproduce with git master
> because commit 61c976c3ccc815aaf0128eea835aee9667cd8bbe reduces flush count)

Looks good to me. There's just a typo in the comment you added r600_context_queries_resume(), should be "doesn't need" instead of "does need".

I think you should send it to the list :)
Comment 36 Pierre-Eric Pelloux-Prayer 2011-07-03 06:23:42 UTC
Created attachment 48706 [details] [review]
GL_SELECT hw support v3

New version !

Changelist :
- corrected the issue with GL_DEPTH_TEST : _mesa_Clear only operates in GL_RENDER mode, so FBO depth buffer wasn't cleared
- minZ and maxZ should be correct now : the FBO is twice as height now. Top row is drawn using GL_LESS depth function, bottom one is drawn using GL_GREATER.

I tried to build another version with custom state seup/teardown hooks to preserve eventual user state changes but ran into massive problems. 
I'm not really sure this is a real issue though, and maybe we can just live it with until this causes a problem in an application.
Comment 37 Lars G 2011-07-03 10:38:39 UTC
Stresstested in Blender 2.58.1 with crash-fix + hw gl_select v2/v3 patches.
Result: Everything is fast and rockstable!
Verry nice! :)
Comment 38 Micael Dias 2011-07-04 02:47:02 UTC
Just tested, and altough very fast, it seems to break grabing/rotating/scaling using the handles on screen. Not sure which patch broke it though...

Anyway, thank you for your work on this :)
Comment 39 Lars G 2011-07-04 04:28:36 UTC
(In reply to comment #38)
> Just tested, and altough very fast, it seems to break grabing/rotating/scaling
> using the handles on screen.

Same here.
Comment 40 Pierre-Eric Pelloux-Prayer 2011-07-04 14:08:09 UTC
Created attachment 48749 [details] [review]
workaround for issue in comment #38 #39

Here's a small patch that seems to workaround the manipulator issue.

I still do not yet understand the exact problem though (it seems z-buffer is not written to but glQueries count is > 0... weird). This will need some further investigation.
Comment 41 Lars G 2011-07-05 15:42:33 UTC
(In reply to comment #40)
> Here's a small patch that seems to workaround the manipulator issue.

Thanks, works ok here.
Comment 42 Pierre-Eric Pelloux-Prayer 2011-07-11 14:58:54 UTC
Created attachment 48987 [details] [review]
GL_SELECT hw support v4

New version !

What's new :
- manipulator issue understood and properly fixed (GL_DEPTH_TEST was disabled by Blender before drawing manipulators)
- query stuff removed : I'm not sure it brought some performance gains, so I removed it for clarity
- user state changes are now correctly preserved, except FBO (if a user bind a new FBO when in GL_SELECT mode, it will either fails with an assert or produces wrong result). I think I'll disable FBO binding when not in GL_RENDER mode (as it is already done in glClear)

What's left before submitting it for merging :
- more testing :-)
- fix Mesa-classic drivers GL_SELECT (mainly Intel). I'd like to enable hw support for them too, but it isn't clear how to do it (yet).
Comment 43 Micael Dias 2011-07-12 02:16:29 UTC
That looks great! Unfortunately I'm unable to test right now as I've been quiet busy.

I'm not sure what's supposed to be the behaviour of changing the FBO while in GL_SELECT mode, but since GL_SELECT doesn't actually write to the FBO, I would assume it would be OK to change FboSave when changing the current FBO while GL_SELECT is active, no?

After that we may need to do some voodoo math to the projection matrix to use the user defined scissor during the render in GL_SELECT. We will also need to intercept scissor changes when in select mode to change the saved scissor and the projection matrix instead of modifying the actual scissor used for rendering which needs to be disabled.

After that I believe we're fully respecting the GL spec and it could possibly be merged into master. Maybe we could add a switch to change from old behaviour to this new behaviour to enable users to test it first before being enabled by default...
Comment 44 Pierre-Eric Pelloux-Prayer 2011-07-12 02:31:41 UTC
(In reply to comment #43)
> I'm not sure what's supposed to be the behaviour of changing the FBO while in
> GL_SELECT mode, but since GL_SELECT doesn't actually write to the FBO, I would
> assume it would be OK to change FboSave when changing the current FBO while
> GL_SELECT is active, no?

Yes, I'll add something like this.

> After that we may need to do some voodoo math to the projection matrix to use
> the user defined scissor during the render in GL_SELECT. We will also need to
> intercept scissor changes when in select mode to change the saved scissor and
> the projection matrix instead of modifying the actual scissor used for
> rendering which needs to be disabled.

According to this : http://glprogramming.com/red/chapter13.html 
"In both feedback and selection modes, information on objects is returned prior to any fragment tests. Thus, objects that would not be drawn due to failure of the scissor, alpha, depth, or stencil tests may still have their data processed and returned in both feedback and selection modes."

So scissor has no effect in GL_SELECT mode, so the current patch looks correct (except it should disable alpha and stencil tests too)

>  Maybe we could add a switch to change from old behaviour
> to this new behaviour to enable users to test it first before being enabled by
> default...

Good idea, will do.
Comment 45 Micael Dias 2011-07-12 04:08:01 UTC
> According to this : http://glprogramming.com/red/chapter13.html 
> "In both feedback and selection modes, information on objects is returned prior
> to any fragment tests. Thus, objects that would not be drawn due to failure of
> the scissor, alpha, depth, or stencil tests may still have their data processed
> and returned in both feedback and selection modes."
> 
> So scissor has no effect in GL_SELECT mode, so the current patch looks correct
> (except it should disable alpha and stencil tests too)
>

Oh, great then :)
Comment 46 Lars G 2011-07-12 13:21:06 UTC
(In reply to comment #42)
> Created an attachment (id=48987) [details]
> GL_SELECT hw support v4

Thanks!
v4 works nice here, no problems found using latest blender cut. :)

I think that the crash-fix.patch is not needed any more because of
http://cgit.freedesktop.org/mesa/mesa/commit/?id=fbe9d4261f94b8a22ae04dccb8201a6762b66d40
Comment 47 Lars G 2011-07-12 14:33:45 UTC
gl_select-hw-v4.patch testing-update:

Issue:
Found an issue with selecting objects displayed as wireframe in object-mode.

Description:
When the object only has a few vertices click-selection works ok, but when the mesh gets dense
(e.g. subdiv modifier on the default cube with level 6) it's nearly impossible to click-select the object.

Notes:
This is only in wireframe view, solid/textured view is *ok*.
This is with mesa-git from 2011-07-09 and only the gl_select-hw-v4.patch applied.
Comment 48 Pierre-Eric Pelloux-Prayer 2011-07-17 11:45:39 UTC
Created attachment 49220 [details] [review]
GL_SELECT hw support v5

Thanks for testing.

I do not have much time available to work on this, so the new version of the patch only fix the selection bug reported in comment #47.

I still plan to modifications mentioned in comment #44 (add as soon as possible).
Comment 49 Lars G 2011-07-17 14:55:11 UTC
(In reply to comment #48)
> GL_SELECT hw support v5

Thanks Pierre-Eric!

v5 fixed the selection issue for me.
Take your time, it's already working verry nice, great work! :)
Comment 50 Lars G 2011-07-30 20:16:09 UTC
Testing notes update:
Tested gl_select.patch v5 for about 2 weeks now in several latest blender and mesa cuts and it's really doing *great*!
No regressions found so far and all is speedy and stable under heavy blender-usage.

Once again, *big thanks!*, i'm really happy! :)
Comment 51 Micael Dias 2011-08-01 20:44:42 UTC
Created attachment 49807 [details] [review]
Implement HW accelerated GL_SELECT

Can someone please test this patch?

It's rewritten from scratch, based on Pierre's last patch but takes a different approach at integrating within current mesa code so that it modifies less code.

To test it, you must export MESA_HW_SELECT before executing the target app.

example:
MESA_HW_SELECT=1 blender


If there are no problems with the patch, I will send it to the dev list.
Comment 52 Lars G 2011-08-02 04:40:28 UTC
(In reply to comment #51)
> Created an attachment (id=49807) [details]
> Implement HW accelerated GL_SELECT
> Can someone please test this patch?

Thanks Micael!

Testing now and found one issue.
I can recreate it by doing the following steps:

Apply an subdiv modifier set to 6 to the default cube.
With wire displayed, all shading modes (matcap/textured/solid) are fast when selecting the object in object-mode.
But once "apply" is clicked in the subdiv modifier the matcap and textured shading modes are slowing down selection, only solid shading mode is still fast.

This is with mesa git 2011-07-30 and recent soc blender from
http://builder.blender.org/download/


Hope this helps!
Comment 53 Micael Dias 2011-08-02 06:27:05 UTC
Thanks for testing.

I can reproduce the problem with normal blender, subdivs applied and textured mode. However it doesn't seem related to this patch or Pierre's since even rotating around the cube is very slow.

I noticed that enabling GLSL mode speeds up rendering and picking remains slow, but I don't think the problem is in this GL_SELECT implementation.

Maybe this is a r600 driver specific problem...
Comment 54 Lars G 2011-08-03 22:06:43 UTC
(In reply to comment #53)
> I don't think the problem is in this GL_SELECT implementation.
Ok, I will do some more testing and report back.
Comment 55 Lars G 2011-08-07 13:47:52 UTC
Did some more testing and it works really great here!
Can't trigger any bugs/regressions/crashes.
So i would say it's ready for primetime! :)
Comment 56 Pierre-Eric Pelloux-Prayer 2011-09-06 10:42:23 UTC
Hi,

I've just found some time to test the 'hw_gl_select' mesa branch.

Test case: r300g, Blender 2.49b (default scene : cube + lamp + camera)

This does not seem to work very well:
 - I'm having some troubles to select objects (mainly the lamp)
 - selection behavior changes between fullscreen/window mode (the camera is almost impossible to select in FS)
(when selection fails zData[0]==1.0 and zData[1]==0.0)

I've tested with this program too: http://sydney.edu.au/engineering/it/~tapted/slow_glselect.html
It doesn't work when MESA_HW_SELECT is set, but works properly without.

This is not the same PC I've tested the earlier patches on, so maybe I've got something wrong on this one.

Lars did you already test this branch on r600g ?
Comment 57 Micael Dias 2011-09-06 16:54:47 UTC
(In reply to comment #56)

I only have a r600 to test. Does your patch work ok or does it also fail?

There are some things I'd like to improve on the implementation like getting rid of most _mesa_* calls to use "more direct" pipe methods instead, but I currently don't have any time to work on it.
Comment 58 Lars G 2011-09-06 19:16:04 UTC
(In reply to comment #56)
> Lars did you already test this branch on r600g ?

I can report that the hw_gl_select-branch works ok here with r600g and blender 2.59.
Comment 59 Séverin Lemaignan 2012-02-25 13:55:07 UTC
Wondering what is the status of this bug.

I'm actually running into something that seem to be the exact same bug with Blender 2.61 on Mesa 7.11/Gallium 0.4 (-> Ubuntu 11.10) with r300g (Radeon Mobility X1600).
Comment 60 Lars G 2012-02-25 14:58:43 UTC
(In reply to comment #59)
> Wondering what is the status of this bug.
> 
> I'm actually running into something that seem to be the exact same bug with
> Blender 2.61 on Mesa 7.11/Gallium 0.4 (-> Ubuntu 11.10) with r300g (Radeon
> Mobility X1600).

i think the patch is still not merged to master.
i'm currently using mesa 7.11.2 (fedora 15) and i had to apply the patch
http://cgit.freedesktop.org/mesa/mesa/commit/?h=hw_gl_select&id=eac31ea1367d1ab1a100513fbbee24a831a8d4e0
from the mesa hw_gl_select branch to make blender stable/usable for me.
i tested the patch for some months now with heavy blender usage and had *not a single issue/crash*, so it would really be nice to merge it imho.

*please, please, pretty please* give us crash-free blendering with mesa drivers 'out of the box'!  :)
Comment 61 hapoofesgeli 2013-07-19 13:27:22 UTC
Hello
Why this bug is still open? i have the same problem  with mesa 9 and blender 2.68...(the patch is for mesa 7 and i couldn't build mesa 7, is it supposed to work with mesa 9?)
I have ATI 5770 + Arch linux...
is there any existing solution?
Comment 62 Alex Deucher 2013-07-19 13:34:49 UTC
Code has not been merged to master yet.  See recent discussion here:
http://lists.freedesktop.org/archives/mesa-dev/2013-June/040102.html

Updated branch here:
http://cgit.freedesktop.org/~ab/mesa/log/?h=hw_gl_select2
Comment 63 hapoofesgeli 2013-07-19 14:07:04 UTC
(In reply to comment #62)
> Code has not been merged to master yet.  See recent discussion here:
> http://lists.freedesktop.org/archives/mesa-dev/2013-June/040102.html
> 
> Updated branch here:
> http://cgit.freedesktop.org/~ab/mesa/log/?h=hw_gl_select2

Thank you
Can you explain that how should i use that updated branch to fix the problem?
Comment 64 Lars G 2013-07-26 10:21:19 UTC
Great this is moving on!
Will test asap.

Thanks! :)
Comment 65 Lars G 2013-08-14 03:07:22 UTC
Tried the new branch now.

Selecting/moving is nice and fast in blender,
but somehow i get no textures displayed on the objects.
This is with radeon hd 6320 (e-450 apu) built against llvm-git, using
blender-git.
Comment 66 Lars G 2013-08-15 17:17:35 UTC
Ok, the missing-texture thing in my last comment was no bug, i only forgot to apply the file-texture correctly in blender.

So after some more testing, hw_gl_select2 works great without any regressions.
Speedy selection of dense meshes, wire and shaded views ok, no selection
problems of other items like lights or cameras.

Verry nice work!
I vote for... upstream! :)
Comment 67 Lars G 2013-08-15 17:59:37 UTC
Created attachment 84111 [details]
Arch Linux PKGBUILD

Attached an Arch Linux PKGBUILD file for easy testing.
Comment 68 Tomasz P. 2013-08-16 11:14:46 UTC
Maybe post this to mesa-dev mailing list.I doubt that they have time to read this.
Comment 69 Alex Deucher 2013-08-16 12:48:57 UTC
I would recommend you do a piglit run (quick driver tests) before and after and make sure there are no regressions.
Comment 70 kao_chen 2013-10-24 22:42:15 UTC
I wonder if the fix has been committed? I am using Debian testing with the mesa 9.1-7 package provided in the testing repository, and the selections with Blender are still very slow.
Comment 71 hapoofesgeli 2013-10-25 07:37:06 UTC
(In reply to comment #70)
> I wonder if the fix has been committed? I am using Debian testing with the
> mesa 9.1-7 package provided in the testing repository, and the selections
> with Blender are still very slow.

It has been merged to master in mesa 9.2.
Comment 72 kao_chen 2013-10-25 09:26:12 UTC
Ok thanks, it seems that mesa 9.2 is approaching Debian testing:
http://packages.qa.debian.org/m/mesa.html
I will wait.
Comment 73 Alex Deucher 2013-10-25 13:11:06 UTC
(In reply to comment #71)
> (In reply to comment #70)
> > I wonder if the fix has been committed? I am using Debian testing with the
> > mesa 9.1-7 package provided in the testing repository, and the selections
> > with Blender are still very slow.
> 
> It has been merged to master in mesa 9.2.

It hasn't been merged yet.
Comment 74 hapoofesgeli 2013-10-25 15:16:55 UTC
S(In reply to comment #73)
> (In reply to comment #71)
> > (In reply to comment #70)
> > > I wonder if the fix has been committed? I am using Debian testing with the
> > > mesa 9.1-7 package provided in the testing repository, and the selections
> > > with Blender are still very slow.
> > 
> > It has been merged to master in mesa 9.2.
> 
> It hasn't been merged yet.

So why i don't have any selection problems?
I'm currently running fedora 19 & mesa 9.2.1 and i don't see those slow selections anymore.
BTW sorry if i was/am wrong.
Comment 75 kao_chen 2013-11-03 09:03:59 UTC
The Mesa packages 9.2.2 arrived yesterday in Debian Testing. I am no longer affected by this bug in Blender. I can switch from a big object to another easily. I have used Blender  more than two hours now, It's working for me.

Thank you very much.
Kao
Comment 76 Thomas131 2013-12-25 20:48:57 UTC
Hello,
Now, I have the same problem with Linux Mint 15. There is also a bug on launchpad.: https://bugs.launchpad.net/ubuntu/+source/blender/+bug/902618
Comment 77 Fabio Pedretti 2015-09-02 07:37:03 UTC
There is a workaround here:
https://bugs.launchpad.net/ubuntu/+source/blender/+bug/902618/comments/6
Comment 78 Kenneth Graunke 2015-09-03 06:07:58 UTC
FWIW, it would be great if that could be made the default in blender - nobody's found time to implement proper GL_SELECT or GL_FEEDBACK support in the last 4 years, and I don't see anybody getting around to it soon.  Blender's literally the only (real) application I know which uses that ancient feature.


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.