Jump to content
The Dark Mod Forums


Recommended Posts

The processed image is only requested once, in GLTextureManager::getBinding(), which calls the TextureConstructor::construct() method. The TextureConstructor allocates the memory for the Image object, writes the pixel data (in whatever complicated way) and returns the readily fabricated Image. The GLTextureManager loads the stuff into OpenGL memory and immediately releases the Image object from memory again. So basically, there is no need to save the actual Image data in any of these MapExpression classes. (Sorry if I'm repeating stuff you might already know :)).


As for changing other classes or interfaces: if you feel that this is necessary, nobody is holding you back. :)


You will probably need to change the implementation of CShader::getBump(), CShader::getDiffuse() and such. Currently, these just create a DefaultConstructor object which loads the actual file from the disk. Instead of

TextureConstructorPtr cons = TextureConstructorPtr(new DefaultConstructor(_bumpName));

this should be

TextureConstructorPtr cons = _bumpMapExpression->getImage();

with _bumpMapExpression as the top-level MapExpression object with all its recursions.


Personally, I would prefer if the MapExpression::getImage() method was named MapExpression::getConstructor(), because it's not delivering an actual image, but a TextureConstructor.


Hopefully, I didn't confuse things more than clarifying them?

Link to comment
Share on other sites

  • Replies 314
  • Created
  • Last Reply

Top Posters In This Topic

Hm, thinking about this, it maybe isn't necessary to keep that TextureConstructor class. If the MapExpression object is capable of delivering the image data, that's fine as well. This could also happen recursively, so there's probably no reason to complicate things by another middleman class.


So, you could probably change the GLTextureManager::getBinding() method to receive an MapExpression object instead of a TextureConstructor.


The call stack would be:

ShaderCache requires bumpmap image >> CShader::getBump() >> GLTextureManager::getBinding(IMapExpress

ionPtr expr) >> calls MapExpression::getImage() >> recursions >> Image is delivered.


[edit] That damn forum software is adding a line break where I don't want it...


Btw: you can probably create a shortcut typedef

typedef boost::shared_ptr<IMapExpression> IMapExpressionPtr;

to save some typing.

Link to comment
Share on other sites

Bypassing the TextureConstructor class would mean returning directly an image object?


Just out of curiosity, would I have had to implement my own version of the textureConstructor class? I only found two implementations that both load from filename.


Is the TextureConstructor class still needed when MapExpressions directly produce image classes?


When the TextureConstructor is only needed for the MapExpressions (that's what it looks like atm), then I would probably remove it.

Link to comment
Share on other sites

Bypassing the TextureConstructor class would mean returning directly an image object?

Yes, the MapExpression class would return the Image.


I think we can't implement the recursive image processing when using TextureConstructors. I thought about a nested addnormals(heightmap(texture/blah), texture/blah2) expression. With the above class inheritance, this would mean that there is one "master" MapExpression containing two child MapExpressions. The master is the one that is contained within the CShader class as _bumpMapExpression and is passed to the GLTextureManager::getBinding(). The GLTextureManager calls getImage() on the master expression, which would have to query its two child objects for their images and to subsequently merge the two textures into one single texture.


At the end of the inheritance chain MapExpressions will of course have to invoke some kind of image loader, but this doesn't have to be encapsulated within a TextureConstructor, like it is now.


Long text, short conclusion: I think we can ditch the TextureConstructor mechanism in favour of nested MapExpression::getImage() methods.


Just out of curiosity, would I have had to implement my own version of the textureConstructor class? I only found two implementations that both load from filename.

Yes, this will be necessary, I guess. There will be need for MapExpressions loading files from the Virtual File System, but some high-level MapExpressions will need to implement or call image processing algorithms. Maybe these algorithms can be outsourced into static methods of a TextureManipulator service class, like the one I attached to the GLTextureManager.

Link to comment
Share on other sites

TextureConstructorPtr cons = _bumpMapExpression->getImage();

Shouldn't that be:

TextureConstructorPtr cons = IMapExpression->createForToken(tok)->getImage();

and now that we discussed it, with an Image instead of Textureconstructor.

Link to comment
Share on other sites

Well, if we ditch that TextureConstructorPtr, it probably should be like this:

Image* (or ImagePtr) img = _bumpMapExpression->getImage();

IMapExpression->getImage() won't work, because there is nothing to dereference here, you'll need a pointer to the class instance, hence _bumpMapExpression, which I implied that it should be declared as a class member of CShader:

class CShader
  IMapExpressionPtr _bumpMapExpression;


edit: Ah, I see the edit now. This won't work either, because you need to call it like this:


(A static method is to be treated like a global function in a namespace, hence IMapExpression::createForToken()).

But I would still vote for saving the IMapExpressonPtr as a local member of CShader, so it can be re-used without having to parse the material files again.

Link to comment
Share on other sites

What do you think is better, first changing the surrounding files to work with the new MapExpressions and then implementing them (makes testing easy) or other way round?


You should make any changes necessary in your local workspace in order to get the implementation into a state where you can test it and confirm it to be working successfully, then use svn diff to produce a patch and send it to us.

Link to comment
Share on other sites

If I were to do it, I would implement a minimal MapExpression implementation first and try to adapt the surroundings to use it (i.e. CShader and GLTextureManager). Whatever feels comfortable to you is good enough.


If you're aiming for a seamless transition you could overload the GLTextureManager::getBinding() method with another variant, taking an IMapExpressionPtr as argument (instead of a TextureConstructor). This way you could use both approaches without breaking the other system. Then try to migrate the CShader::getTexture() method to send IMapExpressionPtrs to the GLTextureManager, using a barebone MapExpression, just loading a file from the disk. Once this is done, you could proceed with making the system more intelligent.


edit: whoah, once again, OrbWeaver was faster. Yes, as soon as you've got a compiling version at hand, we can have a look at it.

Link to comment
Share on other sites

Oh, I see what you mean now -- is it easier to change the calling code to REQUIRE MapExpressions which won't initially work until you implement them, or to implement the MapExpressions first and then splice them into the calling code.


Generally I tend to go with the first approach: decide what you want to happen, then write the code that makes it happen. This is sort of like test-driven development but without the formal processes.

Link to comment
Share on other sites

class IMapExpression {
	   Image result; /* will hold the image on first call of getImage(), further calls to getImage will simply return result */
	   static boost::shared_ptr<IMapExpression> createForToken(parser::DefTokeniser& token);
	   virtual Image& getImage() = 0;


Last question: Is this interface fine now (before I start to change everything to work with this one)?

Link to comment
Share on other sites

Don't put the Image data member as part of the interface, this would be a separate part of each implementing class. Some classes may not even want to store the image, they could create it on demand and return it there and then.


It would also be better if the getImage() method returns a shared_ptr<Image> (typedef'd as ImagePtr) rather than a reference to an existing object.

Link to comment
Share on other sites

The interface is fine, I think.


I'm not sure whether saving the image object as copy is a good idea, because it will moved into OpenGL memory once and after that it's not needed anymore. The GLTextureManager is deleting the image after loading it into OpenGL anyway, so the MapExpression just needs to allocate it, to fill it and to deliver the pointer to it. Otherwise it would stay on the heap (textures can take up several dozens to hundreds of MB quickly).


edit: Argh! Next time I'm gonna wait five minutes before I start typing. :)

Link to comment
Share on other sites

It's best to leave any data members out of the abstract base class definitions, because it only limits the freedom of implementation. It should completely be up to the subclasses how they implement this definition.

Link to comment
Share on other sites

Yep, an interface class should only contain the methods that are part of the public interface for the objects, any private data fields or helper methods should be left out. An interface can sometimes contain constants or typedefs, for example std::string::npos.

Link to comment
Share on other sites

I can't compile it right now, but from reading the code, it looks good. :)


Does this version compile? Because I noticed the getImage() method returns a reference Image&, whereas the actual implementation returns a pointer.


Also, I saw that you're returning this:

return shared_ptr<IMapExpression> p(new MakeAlphaExpression (token));

which could be made simpler, without constructing an intermediate pointer object p:

return shared_ptr<IMapExpression>(new MakeAlphaExpression(token));

I personally would prefer to use a typedef'd IMapExpressionPtr instead of the shared_ptr, but that's not a real concern, more or less a matter of taste.


So far, so good. Did you already have a look at the ShaderFileLoader (the parser)? That's probably the point where these should be hooked in.


In ShaderTemplate.cpp the constructNull function is used twice, should I implement that one too or change ShaderTemplate?

I can't recall right now what this function does, maybe OrbWeaver can tell you more about this?

Link to comment
Share on other sites

I can't recall right now what this function does, maybe OrbWeaver can tell you more about this?


No idea I'm afraid, it's almost certainly legacy cruft that doesn't needs to be there. Properly-designed C++ classes are unlikely to have a function called constructNull(), so there should be no problem removing it if there is a better implementation available.

Link to comment
Share on other sites

greebo: I didn't try to compile it when I posted the links, but since then I'm on fixing compile errors, got quite a lot. Now it compiles. And I also changed some typedefs. About the p in

return shared_ptr<IMapExpression> p(new MakeAlphaExpression (token));

the boost documentation says it's saver.


OrbWeaver: so I'll try to change The ShaderTemplate.


edit: I uploaded the new versions



Link to comment
Share on other sites

The static constructNull method appears to be just a complicated way of allocating an empty MapExpression object (named constructor):

static MapExpressionPtr constructNull() {
		return MapExpressionPtr(new MapExpression());

You can ditch that and initialise the members in ShaderTemplate directly:

_texture(new shaders::MapExpression())

It's likely that this is to circumvent Null-pointer checks somewhere else in the code, so that the assumption _texture != NULL is always true.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Recent Status Updates

    • DeTeEff

      Bachelor Mapping Challenge!
      The girlfriend will be away to her mum's place for almost a week, which means more mapping time for me! I'm planning a speed build. Hope it will go my way
      I'm starting by downloading 2.11. Don't know if I'm going to use any fancy new stuff. Just want to crack those itching map muscles that has gone dry and dead since almost a year's worth of no-mapping
      · 0 replies
    • kano

      The cool part of Canonical's Snap, is that my machine can now ray-trace an entire (simple) scene faster than it takes the web browser to start!
      · 5 replies
    • datiswous

      Fm idea (braindump)
      "Experience the life of a mission builder"
      Esentially there are finnished elements placed somewhere in storage in the game and you have to place them in the correct places, build some walls here and there, add guard routes, give them their correct speech lines, etc. Decorate the places.. all in-game. lots of fun.. 😉
      If you do it right in the end you can play the mission.
      (would be even cooler if a thief-ai could play the mission, making it some kind of tower-defence game)
      This first started as an idea for an aquarium builder mission where-in you have to fill an empty aquarium with sand, waterplants, castles water and swimming fish. But above idea is more fun.
      · 4 replies
    • JackFarmer

      Dear ChatGPT, What are The Builders in the Dark Mod?
      The Builders is a popular game mode in the video game community called "Dark Mod," which is a fan-made tribute to the classic "Thief" series of stealth games. In this game mode, players assume the role of builders who construct structures in a dark and ominous environment while trying to avoid detection by patrolling guards.
      The Builders game mode is unique in that it offers players the opportunity to create and design their own levels using the in-game tools and resources provided. Players can experiment with different building materials, construct intricate traps and puzzles, and create their own unique gameplay experiences.
      While The Builders game mode is not an official part of the Dark Mod, it has become one of the most popular and well-loved aspects of the game, thanks to its emphasis on creativity, strategy, and stealth.
      You guys did not know that, did you?
      · 2 replies
    • The Black Arrow

      I know I'm not active much, but it feels a bit too silent in here. Is everyone taffing around, again?
      · 7 replies
  • Create New...