Skip to content
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

Editing a label, rotate to ~311 degrees, open font menu -> segfault #1093

Closed
chaosphere2112 opened this issue Mar 2, 2015 · 18 comments
Closed
Assignees
Milestone

Comments

@chaosphere2112
Copy link
Contributor

When you edit a text object and rotate it to somewhere around 311 degrees and open the font menu, we segfault.

Discussion on #1081

@doutriaux1
Copy link
Contributor

@dlonie it might be VTK issue so assigning to you.

@chaosphere2112
Copy link
Contributor Author

@dlonie Any update on this? I'm probably going to be doing a workaround for this issue tomorrow if nothing looks promising.

@alliepiper
Copy link
Contributor

@chaosphere2112 I haven't had a chance to look into it yet. Feel free to do a workaround.

chaosphere2112 pushed a commit to chaosphere2112/uvcdat that referenced this issue Mar 9, 2015
@aashish24
Copy link
Contributor

Moving this to 2.3 for now as we have the workaround for 2.2

@aashish24 aashish24 modified the milestones: 2.3, 2.2 Mar 12, 2015
@chaosphere2112
Copy link
Contributor Author

👍

doutriaux1 added a commit that referenced this issue Mar 24, 2015
@alliepiper
Copy link
Contributor

Is this still an issue? I'm trying to reproduce, but can you clarify what you mean by "font menu"? I don't see this. So far I've:

  1. Plotted clt on default boxfill
  2. Clicked the pencil to start editing
  3. Select text
  4. Show angle
  5. Adjust to 311 degrees

Not sure what's next.

@chaosphere2112
Copy link
Contributor Author

@dlonie I removed the font menu (which was a button you could click in the configure menu to open a list of all of the fonts available, styled using those fonts) and replaced with one button you click through to cycle which font you're using. It's not ideal, but it'll work for now. We can look into this further in a week or two.

@alliepiper
Copy link
Contributor

Aha, that would explain it :D

Let me know when this is ready to look into again.

@alliepiper
Copy link
Contributor

Actually, I just triggered a segfault by clicking through the available font faces. I'll see if this is the same one (need to recompile so python's unicode matches my debugger...)

@chaosphere2112
Copy link
Contributor Author

Ok, cool; let me know if you find anything.

@alliepiper
Copy link
Contributor

It is the same crash. Interesting, because the docs say face->size is always initialized for new fonts (and indeed, the freetype sources do this). I'll poke around a bit and see what I can find.

@alliepiper
Copy link
Contributor

Found the issue:

The FreeType library sets face->size to an appropriate FT_Size object when it loads the font face. However, once it is loaded into FreeType's internal caching system, it deletes this object and sets face->size to 0. This modified version is what FreeType gives to VTK when we request a face from the cache manager.

Then we query the kerning information, and an unguarded dereference of face->size occurs, triggering the segfault.

I'm going to put this back on the backburner for now. I have some pending changes to VTK that add DPI awareness to the toolkit, and part of these changes involve rewriting how we access cached font faces. The new implementation actually looks up FT_Size objects in the cache (rather than faces), so we will definitely have valid sizes after that change lands.

I just got the greenlight to work on that feature again, and I should have this in upstream VTK within a few weeks. After that, we can bump VTK/uvcdat-master and this problem should disappear.

I'll leave this bug open so I remember to test then....

@chaosphere2112
Copy link
Contributor Author

Cool! When I did my spelunking, that was my best guess as to the root cause.

@doutriaux1
Copy link
Contributor

@chaosphere2112 should we close that?

@chaosphere2112
Copy link
Contributor Author

@dlonie Did the fix get merged into VTK?

@alliepiper
Copy link
Contributor

@chaosphere2112 It did. Want to try removing your workaround and make sure it's working?

CDAT/VTK@07e728b is the patch that should fix it.

@chaosphere2112
Copy link
Contributor Author

Yup, I'll give this a test and report back.

@doutriaux1
Copy link
Contributor

Fixed by #1514

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

No branches or pull requests

4 participants