Rendering of a press in inscriber leave the GL engine in an improper state #1970

Closed
leagris opened this Issue Oct 25, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@leagris

leagris commented Oct 25, 2015

When displaying a press, the inscriber leave the GL engine in an improper state causing ambient occlusion to not render at some angles.

Look at toolrack render differences (top-right of pictures)
Without press in inscriber:
2015-10-25_23 48 58
With press in inscriber:
2015-10-25_23 49 23

See also:
ForestryMC/ForestryMC#919

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Oct 25, 2015

Member

Anything printed to the log? For example a GL Stackoverflow?

The rendering is actually stupidly over conservative, especially once it needs to render an item (like the press). Like the renderer tries to protect itself from any buggy item render and simultaneously the item renderer protects itself from any buggy caller (which actually is guaranteed AE2), thus really useless. But this easily can result in some stackoverflows etc. Too conservative/defensive is just as bad as too less... Finding an equilibrium is not that easy.

Out of curiositiy, could you try the PR build based on #1880?
It removes most of the extreme defensive stuff and in general is a bit more selective about what state it protects (which should also result in a better performance in some cases, like ridiculuous amounts of skystone chests.
Build against rv3, but it seems you are already using it. But as usually make backups.
Nevertheless it should be mostly finished, just need to test it a bit more (or get some feedback)

Member

yueh commented Oct 25, 2015

Anything printed to the log? For example a GL Stackoverflow?

The rendering is actually stupidly over conservative, especially once it needs to render an item (like the press). Like the renderer tries to protect itself from any buggy item render and simultaneously the item renderer protects itself from any buggy caller (which actually is guaranteed AE2), thus really useless. But this easily can result in some stackoverflows etc. Too conservative/defensive is just as bad as too less... Finding an equilibrium is not that easy.

Out of curiositiy, could you try the PR build based on #1880?
It removes most of the extreme defensive stuff and in general is a bit more selective about what state it protects (which should also result in a better performance in some cases, like ridiculuous amounts of skystone chests.
Build against rv3, but it seems you are already using it. But as usually make backups.
Nevertheless it should be mostly finished, just need to test it a bit more (or get some feedback)

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Nov 11, 2015

Member

While working on this I noticed a few other bugs, this is fixed internally

Member

thatsIch commented Nov 11, 2015

While working on this I noticed a few other bugs, this is fixed internally

@thatsIch thatsIch self-assigned this Nov 11, 2015

@thatsIch thatsIch added this to the rv3 - 1.7.10 milestone Nov 11, 2015

@yueh

This comment has been minimized.

Show comment
Hide comment
@yueh

yueh Nov 11, 2015

Member

Based on my PR or the current master? I had a couple of small rendering issues, mostly lighting based on, which are fixed with the PR. Just to prevent any duplicate work.

Member

yueh commented Nov 11, 2015

Based on my PR or the current master? I had a couple of small rendering issues, mostly lighting based on, which are fixed with the PR. Just to prevent any duplicate work.

@thatsIch

This comment has been minimized.

Show comment
Hide comment
@thatsIch

thatsIch Nov 11, 2015

Member

Always on master, also if it is on your PR you would see your commit in it

Member

thatsIch commented Nov 11, 2015

Always on master, also if it is on your PR you would see your commit in it

@thatsIch thatsIch closed this in 97c099c Nov 16, 2015

yueh added a commit that referenced this issue Nov 16, 2015

Merge pull request #1996 from thatsIch/b-1970-inscriber-lighting
Fixes #1970: Lighting was not re-enabled when looking at an inscriber
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment