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

Hang when changing entity classname from light to info_null #1429

Closed
ericwa opened this issue Sep 7, 2016 · 9 comments
Closed

Hang when changing entity classname from light to info_null #1429

ericwa opened this issue Sep 7, 2016 · 9 comments
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Bug Errors and problems

Comments

@ericwa
Copy link
Collaborator

ericwa commented Sep 7, 2016

  1. Drag a "light" entity from the toolbox in to the map.
  2. Select it and change the classname to "info_null".
  3. Press enter to accept the new classname, TB is frozen

TrenchBroom 2.0.0 Beta Build 2f3c498 RelWithDebInfo / OS X

@ericwa
Copy link
Collaborator Author

ericwa commented Sep 7, 2016

This doesn't hang in the second last Dropbox build, TrenchBroom 2.0.0 Alpha Build 63a040d RelWithDebInfo, but it does hang with the last one, acdfaee

@kduske kduske added Type:Bug Errors and problems Platform:All labels Sep 7, 2016
@kduske kduske added this to the TrenchBroom 2.0.0 milestone Sep 7, 2016
@kduske
Copy link
Collaborator

kduske commented Sep 7, 2016

I can't reproduce this on OS X. Very odd. I even tried it with the exact latest beta built that is available from the website.

@ericwa
Copy link
Collaborator Author

ericwa commented Sep 7, 2016

Weird, I can reproduce it 100% of the time with the beta build from the website.
I'll make a debug build and see what is happening.

@kduske kduske closed this as completed Sep 7, 2016
@kduske kduske reopened this Sep 7, 2016
@kduske
Copy link
Collaborator

kduske commented Sep 7, 2016

Sorry, wrong tab ;-)

@kduske kduske added the Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. label Sep 7, 2016
@ericwa
Copy link
Collaborator Author

ericwa commented Sep 7, 2016

It's the assert(row >= 0 && row < GetRowsCount()); in EntityAttributeGridTable::GetValue that is failing. (EntityAttributeGridTable.cpp:335).

A light entity has 12 rows, and info_null has 4.

From what I can tell, the conversion to an info_null entity went correctly... GetRowsCount() is returning 4, and m_rows also has 4 elements.

For some reason wxWidgets is requesting a value in row 11, here's the backtrace:

  * frame #0: 0x0000000100447105 TrenchBroom`TrenchBroom::View::EntityAttributeGridTable::GetValue(this=0x000060f00014d6a0, row=11, col=1) + 85 at EntityAttributeGridTable.cpp:335
    frame #1: 0x000000010044ab1c TrenchBroom`wxGridTableBase::IsEmptyCell(this=0x000060f00014d6a0, row=11, col=1) + 76 at grid.h:704
    frame #2: 0x0000000103b9500e libwx_osx_cocoau_adv-3.1.dylib`wxGrid::DrawGridCellArea(this=0x000061b0000f5080, dc=0x00007fff5fbfa858, cells=0x00007fff5fbfa820) + 782 at grid.cpp:5410
    frame #3: 0x0000000103b942f1 libwx_osx_cocoau_adv-3.1.dylib`wxGridWindow::OnPaint(this=0x0000617000050f80, (null)=0x00007fff5fbfacc8) + 161 at grid.cpp:1736

@kduske
Copy link
Collaborator

kduske commented Sep 7, 2016

Hmm. Might be related to lazy updates. I think I have changed the code such that it only updates on an idle event, so it's possible that wxWidgets is working with stale data in the paint handler. Still doesn't explain why it's not failing for me.

@ericwa
Copy link
Collaborator Author

ericwa commented Sep 7, 2016

Maybe try reproducing on a debug build? Since those assertions are not compiled into the release builds, there will be an out-of-bounds array access. So it could be failing for you, but just due to luck, not crashing.

@kduske
Copy link
Collaborator

kduske commented Sep 7, 2016

I did try it with a debug build. But I figured it out: wxWidgets will only ask for those lines if they are visible. But in my window layout, only the first three lines were visible ;-)

kduske added a commit that referenced this issue Sep 8, 2016
…s when the grid wants to paint and has outdated information about the number of rows.
@kduske
Copy link
Collaborator

kduske commented Sep 8, 2016

Fixed in bf48380.

@kduske kduske closed this as completed Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prio:2 Medium priority: Non crash bugs that impede the user, features that add new functionality. Type:Bug Errors and problems
Projects
None yet
Development

No branches or pull requests

2 participants