New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mrview Connectome tool: Streamtubes not displaying #1330

Open
Lestropie opened this Issue May 9, 2018 · 19 comments

Comments

Projects
None yet
3 participants
@Lestropie
Member

Lestropie commented May 9, 2018

A continuation of #1311; re-listing as the two bugs originally reported appear to in fact be unrelated, and so need a "clean sheet" to investigate the second.

Primarily reported on the forum here, but also mentioned during user testing of the node mesh visualisation problem here.

Attempting to visualise edges as streamtubes within the mrview Connectome tool results in either no edges being displayed, or an outright mrview crash. May be specific to Max OSX.

If any devs can reproduce this it would be greatly appreciated.

@LucSam

This comment has been minimized.

LucSam commented May 16, 2018

Hi, I just run the debugger on one of our macs and revceived following error.

Edit: The error occurred after changing from streamlines to to streamtubes:

LLDB.txt

(Edit: Uploaded LLDB output as text file for preservation of sanity - Rob)

@Lestropie

This comment has been minimized.

Member

Lestropie commented May 18, 2018

Thanks for the input @LucSam.

Okay, that's ... awkward. The problematic line is here: specifically statement "assert (streamtube);" fails. Data for this class should be allocated by the following stack:

Since this all looks as it should, I can only suggest that this must be some sort of GL update timing problem with Macs: Something is invoking updateGL() before function Connectome::get_streamtubes() has completed, and hence the data required for that graphical update have not yet been allocated. @jdtournier Would you agree? Though I'd have thought that such behaviour would be causing more widespread issues in mrview given our extensive use of resource-allocation-on-demand.

@LucSam: One thing you could try is replacing file src/gui/mrview/tool/connectome/edge.h, line 60:

void render_streamtube() const { assert (streamtube); streamtube->render(); }

with:

void render_streamtube() const { if (!streamtube) return; streamtube->render(); }

Then, see if after some period of time the display starts working correctly.

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

Dear Rob,
You're welcome, I'm glad I can help.
I just changed line 60 in edge.h which results in:

mrview: [SYSTEM FATAL CODE: SIGSEGV (11)] Segmentation fault: Invalid memory access

I can run it later on a debugger if that helps more.

@Lestropie

This comment has been minimized.

Member

Lestropie commented May 18, 2018

Hmmm. I'd expected that bypassing that draw call would have been sufficient: I don't see anywhere else in the code where the streamtube data are accessed without verification. There might be something even more exotic going on, which unfortunately I have no hope of figuring out without some more debugger action... 🤞

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

@Lestropie
Compiling MRtrix3 in debug mode failed because of the changed line 60:

debugger_fail_line60_changed.txt

@Lestropie

This comment has been minimized.

Member

Lestropie commented May 18, 2018

It looks like you've retained the original line of code in addition to adding the new one, rather than replacing the line. There should only be one line of code that begins with "void render_streamtube()". It's possible that depending on your exact version of MRtrix3, the line number is not precisely accurate; the more important thing is to replace the relevant code.

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

oops.
So, after replacing the correct line..: There's no error showing up with lldb (and still no streamtubes).

This is the output of valgrind:

valgrind.txt

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 18, 2018

OK, I have to admit I'm not familiar with what's going on here - it's not my code... 😜

But looking through the code you point to, it strikes me that given this line, we should see a line with Generating connection streamtubes somewhere in the output - and I don't see it. So it looks like it's not even getting to Connectome::get_streamtubes(). Looking at where it's supposed to be invoked, it looks like this only happens if the variable Connectome::have_streamtubes is false. But a quick grep suggests that this variable is never initialised...

$ grep -rn have_streamtubes .
./src/gui/mrview/tool/connectome/connectome.cpp:1762:                if (!have_streamtubes) {
./src/gui/mrview/tool/connectome/connectome.cpp:3832:          have_streamtubes = true;
./src/gui/mrview/tool/connectome/connectome.h:330:            bool have_exemplars, have_streamtubes;

None of these occur anywhere near the constructor...

@LucSam, I think you need to add an explicit initialisation at line 85 in the file src/gui/mrview/tool/connectome/connectome.cpp, so it looks like this:

            ...
            have_exemplars (false),
            have_streamtubes (false),
            edge_fixed_colour { 0.5f, 0.5f, 0.5f },
            ...

That sounds like the most likely explanation to me...

Note that valgrind would have caught that, but it's poorly supported on macOS, and indeed didn't get very far at all in this case, bailing out when an unknown instruction was encountered... But given the code as it is, I'm guessing we should pick this up on Linux valgrind too.

Also of note: this is an AMD/ATI system, and their drivers are always more picky. You often get issues with the OpenGL handling not being exactly to standard, and it's rarely all that obvious when it isn't... But it doesn't look like this is the issue here (at least the outright segfault isn't...).

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

@jdtournier
thanks for the notes on valgrind and amd/ati systems.

Unfortunately, inserting have_streamtubes (false), didn't solve the problem..

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 18, 2018

Ok, when you say it didn't solve the problem, what exactly do you mean by that? Does it still crash out with a segmentation fault at the same point with the same assert failure? Or does it simply not render the streamtubes?

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

Sorry for being unspecific.
It doesn't render the streamtubes.

Edit: The crashing didn't occur anymore.

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 18, 2018

No need to apologise - it's hard to second-guess how our brains work...

So that's good news, in that at least that's one issue sorted. Can I check that this now runs without crashing or assert failures within the lldb session, using a -assert -debug config? If so, then we can start looking into the OpenGL handing as the prime suspect...

@jdtournier jdtournier closed this May 18, 2018

@jdtournier jdtournier reopened this May 18, 2018

@LucSam

This comment has been minimized.

LucSam commented May 18, 2018

Yes, I double checked: There is no crashing and no assert failure with lldbanymore.

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 18, 2018

Ok, thanks for confirming. We'll need to go over the OpenGL code and see if there's any issues like #1130...

Lestropie added a commit that referenced this issue May 21, 2018

mrview Connectome tool: Fix streamtube initialisation
Failure to initialise member bool Connectome::have_streamtubes could lead to memory issues, as reported in #1330.
@Lestropie

This comment has been minimized.

Member

Lestropie commented May 21, 2018

Damnit, I was intentionally not providing a link to #1130; but now you've gone and dunnit... 😅

If any devs on Mac OSX (esp. with ATI) could test out this code and see if they can reproduce, it would be very much appreciated.

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 21, 2018

Damnit, I was intentionally not providing a link to #1130; but now you've gone and dunnit... 😅

Had to be done...

On the plus side: this probably means the issue occurs on any ATI systems, macOS or not. We've got plenty of those in my department. Is there any test data for me to have a play with?

@Lestropie

This comment has been minimized.

Member

Lestropie commented May 21, 2018

this probably means the issue occurs on any ATI systems, macOS or not.

Not sure; Was reported here on a GTX1060. But info is a little scattered due to the conflation with the node mesh visualisation issue.

Is there any test data for me to have a play with?

Test data from #1311 still applies.

@jdtournier

This comment has been minimized.

Member

jdtournier commented May 22, 2018

Yes, hard to tell whether the issue on that NVIDIA system was purely locale-related. The user reports that things work, but might talking purely about the mesh issue. I guess we should ask them to clarify...

@Lestropie

This comment has been minimized.

Member

Lestropie commented Jul 3, 2018

Reported on forum: Rendering edges as cylinders works, but streamtubes do not. It is a significantly different rendering path, but maybe there's a hint in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment