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

ncplane_mergedown() isn't working for small target planes #417

Closed
dankamongmen opened this issue Mar 22, 2020 · 6 comments
Closed

ncplane_mergedown() isn't working for small target planes #417

dankamongmen opened this issue Mar 22, 2020 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@dankamongmen
Copy link
Owner

dankamongmen commented Mar 22, 2020

Having added a background to the Tetris demo, I've made the playing area its own plane above the background. Attempting to ncplane_mergedown() settled pieces onto this plane, however, is running
into problems. I think this is due to continued use of lfdimx within paint() -- if paint() is our method of implementing ncplane_mergedown() (which it really does need to be), it oughtn't be using any parameters from notcurses* nc. Anywhere that it is is almost certainly a bug.

Don't call this "fixed" without a good unit test on it.

@dankamongmen dankamongmen added the bug Something isn't working label Mar 22, 2020
@dankamongmen dankamongmen added this to the 1.3.0 milestone Mar 22, 2020
@dankamongmen dankamongmen self-assigned this Mar 22, 2020
@dankamongmen
Copy link
Owner Author

I've added a unit test in dankamongmen/tetris, which is crashing, so huzzah!

@dankamongmen
Copy link
Owner Author

Oh weird this might be due to ncplane_polyfill_recurse()? According to valgrind anyway. That seems surprising:

==1390854== Command: ./notcurses-tester --tc=Fills --sc=MergeDownSmallPlane
==1390854== 
==1390854== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==1390854== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==1390854== Can't extend stack to 0x1ffe8010b8 during signal delivery for thread 1:
==1390854==   no stack segment
==1390854== 
==1390854== Process terminating with default action of signal 11 (SIGSEGV)
==1390854==  Access not within mapped region at address 0x1FFE8010B8
==1390854== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==1390854==    at 0x485F9F7: cell_simple_p (notcurses.h:1745)
==1390854==    by 0x48604CF: pool_release (internal.h:571)
==1390854==    by 0x486052A: cell_duplicate_far (internal.h:580)
==1390854==    by 0x4860955: cell_duplicate (render.c:95)
==1390854==    by 0x484F04C: ncplane_polyfill_recurse (fill.c:34)
==1390854==    by 0x484F07A: ncplane_polyfill_recurse (fill.c:38)
==1390854==    by 0x484F0AC: ncplane_polyfill_recurse (fill.c:42)
==1390854==    by 0x484F07A: ncplane_polyfill_recurse (fill.c:38)
==1390854==    by 0x484F0AC: ncplane_polyfill_recurse (fill.c:42)
==1390854==    by 0x484F07A: ncplane_polyfill_recurse (fill.c:38)
==1390854==    by 0x484F0AC: ncplane_polyfill_recurse (fill.c:42)
==1390854==    by 0x484F07A: ncplane_polyfill_recurse (fill.c:38)
==1390854==  If you believe this happened as a result of a stack
==1390854==  overflow in your program's main thread (unlikely but
==1390854==  possible), you can try to increase the size of the
==1390854==  main thread stack using the --main-stacksize= flag.
==1390854==  The main thread stack size used in this run was 8388608.
==1390854== Stack overflow in thread #1: can't grow stack to 0x1ffe801000
==1390854== 

@dankamongmen
Copy link
Owner Author

I ramped the polyfill unit tests up without trouble, so I think this might be a red herring.

dankamongmen added a commit that referenced this issue Mar 22, 2020
@dankamongmen
Copy link
Owner Author

dankamongmen commented Mar 23, 2020

Ahhh, the polyfill() thing was that I was passing a null EGC, which yeah that won't work LOL. We ought guard against that. I've created #418 for this.

@dankamongmen
Copy link
Owner Author

Well shit, with #418 resolved, the unit test now passes :/. And yet things are not working in tetris...

@dankamongmen
Copy link
Owner Author

Resolved in dankamongmen/tetris. As predicted, we just needed to eliminate all uses of nc in paint(). I've done that, and the behavior in tetris is fixed.

dankamongmen added a commit that referenced this issue Mar 23, 2020
* ncplane_mergedown() fix for small planes #417
* tetris game needed for the book
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant