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

Enable shaders for CAOs. #1971

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@RealBadAngel
Copy link
Contributor

commented Dec 14, 2014

screenshot_1211176161
screenshot_1211772702

@@ -806,6 +809,15 @@ void GenericCAO::addToScene(scene::ISceneManager *smgr, ITextureSource *tsrc,
m_smgr = smgr;
m_irr = irr;

m_enable_shaders = g_settings->getBool("enable_shaders");

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja Dec 15, 2014

Member

This should be cached. It's stored in a member but the member is updated on every run.

EDIT: Seems like this is a sort of late constructor, so this might be O.K.

@PilzAdam

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2014

http://irc.minetest.ru/minetest-dev/2014-12-15#i_4064461
Yes, the feature freeze applies. Also my experience is that graphic related changes tend to break something for somebody, do merging this so close before a release isn't good.

@RealBadAngel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2014

@PilzAdam this is the very same kind of change we have applied to wielded, so it is safe.
This is very trivial change, just test it.

@PilzAdam

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2014

@RealBadAngel well, thats just another reason not to merge this (#1860).

@RealBadAngel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2014

Huh? thats not even related!
Wielded is mostly kahrl's code, shaders are mine. ATM i dont have a slightest idea why mesh is fully bright always. Kahrl seems to have tried to make it forward compatible with hardware lighting and i wasnt expecting that.
Anyway, im reverting my approval for screwdriver changes. Fix the issues i mentioned before askin my vote again.

@kwolekr kwolekr force-pushed the minetest:master branch to 3f83ca2 Dec 25, 2014

@kwolekr

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2014

This can't be merged until pointers are properly validated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.