Isolate irrlicht references and use a singleton #6041

nerzhul · 2017-06-24T14:39:41Z

Isolate irrlichtdevice to a RenderingEngine class, with various static function related to the device singleton

This permits to remove many function cost passing irrlicht or scene_mgr or videodriver and isolate minetest code from irrlicht interfaces.
This is a first step needed to minetest to properly have a rendering interface permitting to use another rendering engine, if needed.

move getSupportedVideoDrivers to Device3D Add Device3D singleton & use it in various places Rename Device3D to Rendering engine & add helper functions to various device pointers More singleton work RenderingEngine owns draw_load_screen move draw functions to RenderingEngine Reduce IrrlichtDevice exposure and guienvironment RenderingEngine: Expose get_timer_time() to remove device from guiEngine Make irrlichtdevice & scene manager less exposed

Dumbeldor · 2017-06-24T14:54:29Z

src/client/renderingengine.h

+	void init_texture(const v2u32& screensize,
+		video::ITexture** texture, const char* name);
+
+	video::ITexture* draw_image(const v2u32 &screensize,


video::ITexture *draw_image

video::ITexture *drawImage

Dumbeldor · 2017-06-24T14:58:13Z

src/client/renderingengine.h

+
+	void draw_pageflip_3d_mode(Camera *camera, bool show_hud,
+		Hud *hud, const v2u32 &screensize, bool draw_wield_tool, Client *client,
+		gui::IGUIEnvironment* guienv, const video::SColor &skycolor);


gui::IGUIEnvironment *guienv

Dumbeldor · 2017-06-24T14:58:38Z

src/client/renderingengine.h

+		const video::SColor &skycolor);
+
+	void init_texture(const v2u32& screensize,
+		video::ITexture** texture, const char* name);


video::ITexture **texture, const char *name

Dumbeldor · 2017-06-24T14:58:57Z

src/client/renderingengine.h

+		Camera *camera, Hud *hud, bool draw_wield_tool, Client *client,
+		gui::IGUIEnvironment *guienv, const video::SColor &skycolor);
+
+	video::ITexture* draw_hud(const v2u32 &screensize,


Dumbeldor · 2017-06-24T14:59:13Z

src/client/tile.cpp

@@ -238,15 +239,15 @@ class SourceImageCache
 		return NULL;
 	}
 	// Primarily fetches from cache, secondarily tries to read from filesystem
-	video::IImage* getOrLoad(const std::string &name, IrrlichtDevice *device)
+	video::IImage* getOrLoad(const std::string &name)


Dumbeldor · 2017-06-24T14:59:20Z

src/client/tile.cpp

 	{
 		std::map<std::string, video::IImage*>::iterator n;
 		n = m_images.find(name);
 		if (n != m_images.end()){
 			n->second->grab(); // Grab for caller
 			return n->second;
 		}
-		video::IVideoDriver* driver = device->getVideoDriver();
+		video::IVideoDriver* driver = RenderingEngine::get_video_driver();


Dumbeldor · 2017-06-24T15:06:37Z

src/client/tile.cpp

@@ -435,16 +428,13 @@ class TextureSource : public IWritableTextureSource
 	bool m_setting_anisotropic_filter;
 };

-IWritableTextureSource* createTextureSource(IrrlichtDevice *device)
+IWritableTextureSource* createTextureSource()


Dumbeldor · 2017-06-24T15:06:47Z

src/client/tile.cpp

@@ -461,7 +451,7 @@ TextureSource::TextureSource(IrrlichtDevice *device):

 TextureSource::~TextureSource()
 {
-	video::IVideoDriver* driver = m_device->getVideoDriver();
+	video::IVideoDriver* driver = RenderingEngine::get_video_driver();


Dumbeldor · 2017-06-24T15:06:55Z

src/client/tile.cpp

 	m_source_image_existence.set(name, true);
 }

 void TextureSource::rebuildImagesAndTextures()
 {
 	MutexAutoLock lock(m_textureinfo_cache_mutex);

-	video::IVideoDriver* driver = m_device->getVideoDriver();
+	video::IVideoDriver* driver = RenderingEngine::get_video_driver();


Dumbeldor · 2017-06-24T15:07:06Z

src/client/tile.cpp

@@ -1196,13 +1181,13 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		video::IImage *& baseimg)
 {
 	const char escape = '\\'; // same as in generateImage()
-	video::IVideoDriver* driver = m_device->getVideoDriver();
+	video::IVideoDriver* driver = RenderingEngine::get_video_driver();


Dumbeldor · 2017-06-24T15:07:14Z

src/client/tile.cpp

@@ -1275,15 +1260,15 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 				blit_with_alpha(image, baseimg, pos_from, pos_to, dim);
 			} else if (dim.Width * dim.Height < dim_dst.Width * dim_dst.Height) {
 				// Upscale overlying image
-				video::IImage* scaled_image = m_device->getVideoDriver()->
+				video::IImage* scaled_image = RenderingEngine::get_video_driver()->


Dumbeldor · 2017-06-24T15:12:25Z

src/particles.h

-	void addNodeParticle(IGameDef* gamedef, scene::ISceneManager* smgr,
-		LocalPlayer *player, v3s16 pos, const MapNode &n,
-		const ContentFeatures &f);
+	void addNodeParticle(IGameDef* gamedef, LocalPlayer *player, v3s16 pos,


Dumbeldor · 2017-06-24T15:12:46Z

src/script/lua_api/l_mainmenu.cpp

@@ -843,7 +839,7 @@ int ModApiMainMenu::l_extract_zip(lua_State *L)
 	if (ModApiMainMenu::isMinetestPath(absolute_destination)) {
 		fs::CreateAllDirs(absolute_destination);

-		io::IFileSystem* fs = engine->m_device->getFileSystem();
+		io::IFileSystem* fs = RenderingEngine::get_filesystem();


Dumbeldor · 2017-06-24T15:13:00Z

src/shader.cpp

@@ -332,17 +331,16 @@ class ShaderSource : public IWritableShaderSource
 	std::vector<ShaderCallback *> m_callbacks;
 };

-IWritableShaderSource* createShaderSource(IrrlichtDevice *device)
+IWritableShaderSource* createShaderSource()


Dumbeldor · 2017-06-24T15:13:34Z

src/shader.cpp

 }

 /*
 	Generate shader given the shader name.
 */
 ShaderInfo generate_shader(const std::string &name,
-		u8 material_type, u8 drawtype,
-		IrrlichtDevice *device, std::vector<ShaderCallback *> &callbacks,
+		u8 material_type, u8 drawtype, std::vector<ShaderCallback *> &callbacks,
 		const std::vector<IShaderConstantSetterFactory*> &setter_factories,


Dumbeldor · 2017-06-24T15:13:58Z

src/shader.cpp

 	}

 	bool enable_shaders = g_settings->getBool("enable_shaders");
 	if (!enable_shaders)
 		return shaderinfo;

-	video::IVideoDriver* driver = device->getVideoDriver();
-	sanity_check(driver);
+	video::IVideoDriver* driver = RenderingEngine::get_video_driver();


Dumbeldor

LGTM

sfan5 · 2017-06-24T22:22:32Z

src/client/renderingengine.h

+	static void setXorgClassHint(const video::SExposedVideoData &video_data,
+		const std::string &name);
+	bool setWindowIcon();
+	bool setXorgWindowIconFromPath(const std::string &icon_file);


why are these part of the rendering engine?
IMO porting makes more sense

setXorgWindowIconFromPath uses Irrlicht:

video::IVideoDriver *v_driver = m_device->getVideoDriver(); video::IImageLoader *image_loader = NULL;

then it make sense to link it with rendering engine

nerzhul · 2017-06-25T07:27:07Z

@sfan5 i don't know if it's really non trivial, it's a huge rework it's right, but the change itself in all engine is not difficult it just take some time :p

EDIT: also note i cleanup many IrrlichtDevice references, VideoDriver, SceneManager and guiEnvironment but all it not done yet, i prefer cleanup other parts after making this PR merged to ensure the work is okay

nerzhul · 2017-06-25T12:25:54Z

Okay PR is ready for merge if CI pass. @sfan5 @SmallJoker @Zeno- please do a review

…layDensity, getDisplaySize to RenderingEngine Fix XORG_USED macro -> RenderingEngine + create_engine_device from RenderingEngine constructor directly

SmallJoker · 2017-06-25T13:18:57Z

src/client/renderingengine.h

+		return s_singleton->m_device->getGUIEnvironment();
+	}
+
+	static void draw_load_screen(const std::wstring &text,


Why inline draw_scene but not here too?

because function is not inlined itself, it's split between declaration & definition

pilino1234 · 2017-06-25T13:06:58Z

src/client/renderingengine.h

+	static std::vector<irr::video::E_DRIVER_TYPE> getSupportedVideoDrivers();
+
+private:
+	enum paralax_sign


parallax_sign

pilino1234 · 2017-06-25T13:52:58Z

src/client/renderingengine.cpp

+	irr::core::vector3df eye_pos;
+	irr::core::matrix4 movement;
+	movement.setTranslation(irr::core::vector3df(
+			(int)psign * g_settings->getFloat("3d_paralax_strength"), 0.0f,


s/paralax/parallax over the whole file 😄

it's not a new setting but an old setting, this is not for this PR, please do a fix PR after merging this

I fixed the enum as it's a private enum, but i don't modify the setting name which is out of this PR scope

Okay, I'll do that

pilino1234 · 2017-06-25T13:58:04Z

src/client/renderingengine.h

+	void init_texture(const v2u32 &screensize, video::ITexture **texture,
+			const char *name);
+
+	video::ITexture *draw_image(const v2u32 &screensize, paralax_sign psign,


parallax_sign psign for spelling fix

pilino1234 · 2017-06-25T13:58:33Z

src/client/renderingengine.cpp

+			irr::video::ECF_A8R8G8B8);
+}
+
+video::ITexture *RenderingEngine::draw_image(const v2u32 &screensize, paralax_sign psign,


parallax_sign psign

SmallJoker

Tested, works.

* Add Device3D class which will contain IrrlichtDevice interface move getSupportedVideoDrivers to Device3D Add Device3D singleton & use it in various places Rename Device3D to Rendering engine & add helper functions to various device pointers More singleton work RenderingEngine owns draw_load_screen move draw functions to RenderingEngine Reduce IrrlichtDevice exposure and guienvironment RenderingEngine: Expose get_timer_time() to remove device from guiEngine Make irrlichtdevice & scene manager less exposed * Code style fixes * Move porting::getVideoDriverName, getVideoDriverFriendlyName, getDisplayDensity, getDisplaySize to RenderingEngine Fix XORG_USED macro -> RenderingEngine + create_engine_device from RenderingEngine constructor directly * enum paralax => enum parallax

nerzhul added the Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements label Jun 24, 2017

nerzhul force-pushed the irrlicht_isolation branch from 6bd4647 to 1360f3c Compare June 24, 2017 14:45

Dumbeldor suggested changes Jun 24, 2017

View reviewed changes

Dumbeldor approved these changes Jun 24, 2017

View reviewed changes

minetest deleted a comment from Dumbeldor Jun 24, 2017

sfan5 reviewed Jun 24, 2017

View reviewed changes

sfan5 added the Non-trivial A large amount of work is required to address this (sometimes to the point of being infeasible). label Jun 24, 2017

nerzhul force-pushed the irrlicht_isolation branch from c08c72f to b801b81 Compare June 25, 2017 08:00

Code style fixes

93d9932

nerzhul force-pushed the irrlicht_isolation branch from b801b81 to 93d9932 Compare June 25, 2017 11:41

Move porting::getVideoDriverName, getVideoDriverFriendlyName, getDisp…

4fa3b1c

…layDensity, getDisplaySize to RenderingEngine Fix XORG_USED macro -> RenderingEngine + create_engine_device from RenderingEngine constructor directly

nerzhul force-pushed the irrlicht_isolation branch from 5d0386d to 4fa3b1c Compare June 25, 2017 12:57

SmallJoker reviewed Jun 25, 2017

View reviewed changes

pilino1234 reviewed Jun 25, 2017

View reviewed changes

nerzhul added 2 commits June 25, 2017 16:33

enum paralax => enum parallax

69e5e67

inline draw_load_screen

ac73810

minetest deleted a comment from pilino1234 Jun 25, 2017

SmallJoker approved these changes Jun 26, 2017

View reviewed changes

SmallJoker added the One approval ✅ ◻️ label Jun 26, 2017

nerzhul merged commit b3a36f7 into minetest:master Jun 26, 2017

nerzhul deleted the irrlicht_isolation branch August 19, 2017 10:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Isolate irrlicht references and use a singleton #6041

Isolate irrlicht references and use a singleton #6041

nerzhul commented Jun 24, 2017

Dumbeldor Jun 24, 2017

SmallJoker Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor Jun 24, 2017

Dumbeldor left a comment

sfan5 Jun 24, 2017

nerzhul Jun 25, 2017

nerzhul commented Jun 25, 2017 •

edited

nerzhul commented Jun 25, 2017

SmallJoker Jun 25, 2017

nerzhul Jun 25, 2017

pilino1234 Jun 25, 2017

pilino1234 Jun 25, 2017

nerzhul Jun 25, 2017

nerzhul Jun 25, 2017

pilino1234 Jun 25, 2017

pilino1234 Jun 25, 2017

pilino1234 Jun 25, 2017

SmallJoker left a comment

Isolate irrlicht references and use a singleton #6041

Isolate irrlicht references and use a singleton #6041

Conversation

nerzhul commented Jun 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dumbeldor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nerzhul commented Jun 25, 2017 • edited

nerzhul commented Jun 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SmallJoker left a comment

Choose a reason for hiding this comment

nerzhul commented Jun 25, 2017 •

edited