mohij Posted May 23, 2007 Report Share Posted May 23, 2007 The MapExpressions are only implemented on a very basic level at the moment. They should be reimplemented in a better way. At the moment I need to understand how they are used (or should be used) and how the new design should look. OrbWeaver said something about a hierarchy of polymorphic objects and an IMapExpression interface.The IMapExpression interface sounds like a module, I'm not sure whether a completely new module is appropriate for such a special thing as MapExpressions.Polymorphic objects sound good, but I'm not sure how the hierarchy should be done, the current implementation seems a bit trival, perhaps there is a better solution. Is the functionality of the MapExpressions (addnormals, hightmap, etc.) implemented somewhere already, or do I have to write those from scratch too? So that's where I currently am, tell me what you think. Quote Link to comment Share on other sites More sharing options...
greebo Posted May 23, 2007 Report Share Posted May 23, 2007 OrbWeaver knows best about the current MapExpressions hierarchy, I know more about the ShaderSystem itself, which is surrounding all that stuff. As for the functionality: the algorithm for addnormals() is nowhere implemented so far (this will need some serious thinking and/or googling I guess), but I have saved a heightMapCreator code snippet in the shaders module, which looks functional (haven't tested it). Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted May 23, 2007 Report Share Posted May 23, 2007 OrbWeaver said something about a hierarchy of polymorphic objects and an IMapExpression interface.The IMapExpression interface sounds like a module, I'm not sure whether a completely new module is appropriate for such a special thing as MapExpressions.Polymorphic objects sound good, but I'm not sure how the hierarchy should be done, the current implementation seems a bit trival, perhaps there is a better solution. All modules have an interface, but not all interfaces are modules. I mean an interface similar to the following (within the shaders module): class IMapExpression { public : virtual ImageConstructor& getImage() = 0; } Where ImageConstructor is the lazy-evaluating image construction object that Greebo implemented in the shaders module. Individual map expression types would then implement this interface, so that you would have a HeightMapExpression, an AddNormalsMapExpression and so on. When the getImage method was called, the map expression subclass would recursively evaluate any "child" IMapExpressions it contained, which in most cases would be a simple texture name but could also include any number of recursive calls to addnormals() etc, and then perform its own class-specific image processing on the results in order to return its own "flattened" image. How best to construct the parse tree requires some consideration too, the process is based around a stream of tokens retrieved from a parser::DefTokeniser object. I am not immediately sure whether each MapExpression subclass should construct itself from a token stream, or whether there should be a separate service class which reads the token stream and constructs the relevant MapExpressions explicitly. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
mohij Posted May 23, 2007 Author Report Share Posted May 23, 2007 I think an extra "parsing class" that creates the MapExpressions would be better. I think it would be easier to handle as things are seperated. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 24, 2007 Author Report Share Posted May 24, 2007 After thinking about a service class, I am not sure anymore whether it would be easier with an extra service class.This extra service class would have to seperate between the different Tokens and work differently for different MapExpressions (as the different MapExpressions need different input).So I think implementing a service class that differenciates between the different MapExpressions and parses them the input they need would end in one big service class that does all the work and very small MapExpression classes that could as well be only functions. (the polymorphism wouldn't get used). MapExpressions that call each other are perhaps still better. Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted May 24, 2007 Report Share Posted May 24, 2007 Well you're going to need some kind of factory class obviously, in order to create the correct MapExpression class from a given token, but you are correct that the MapExpressions will need to parse their own token stream, otherwise as you say the logic will all be in the service class and the MapExpressions won't really do anything. You could have a static method in the base class, e.g. class IMapExpression { public: boost::shared_ptr<IMapExpression> createForToken(const std::string& token); } which would look up the string token in a static std::map and return a new instance of the correct MapExpression. The parsing code in each MapExpression would then need to detect that there was a recursive expression inside it, and call this function to create an instance of the required class. This means that for parsing the expression map addnormals(textures/blah, heightmap(textures/bleh)) the sequence would be 1. Shaders code detects the "map" keyword which introduces a map expression, then parses the next token.2. Token is passed to the static factory method to create the required MapExpression. It looks in its map and constructs an AddNormalsExpression which is returned to the shaders code.3. Shaders code passes the tokeniser to AddNormalsExpression.4. AddNormals expression reads the "(".5. AddNormals expression reads the next token, and detects it is a texture. This is added as the first child for later processing (you might want an ImageMapExpression that is another subclass of IMapExpression representing a single texture name).6. AddNormalsExpression reads the ",".7. The next token is heightmap, which requires a new MapExpression. The factory method is called once again, and returns a HeightMapExpression.8. AddNormalsExpression passes the current tokeniser to the new HeightMapExpression.9. HeightMapExpression parses the bracket, texture name and bracket. It adds the texture as its single permitted child.10. The construction of HeightMapExpression is complete. Control returns to the AddNormalsExpression which parses the final ")".11. Construction of AddNormalsExpression is complete. Control returns to the shaders module, which now has a complete parse tree representing the map expression in its entirety.12. Shaders module continues where it left off, reading tokens from the Tokeniser. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
mohij Posted May 24, 2007 Author Report Share Posted May 24, 2007 With such a createForToken function I wouldn't even need an extra factory class. The method would simply be called for the base class by the Shader code, that's cool.And I think that this single function should already suffice as the public interface for the class.I will try/start to implement it tomorrow, I guess the next big task will be the MapExpressions themselves. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 25, 2007 Author Report Share Posted May 25, 2007 Would aboost::shared_ptr<IMapExpression> createForToken(parser::DefTokeniser& tok) external friend function also be acceptable instead of another class? I got stuck while trying to use this function as a member of class IMapExpression, because you can't instanciate a class that is has vitual, but need to, because the createForToken is member of it. So simplest solution would be to make it external, but friend. Is that acceptable? Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted May 25, 2007 Report Share Posted May 25, 2007 The method needs to be static, so that it is not associated with a specific class instance. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
mohij Posted May 25, 2007 Author Report Share Posted May 25, 2007 I want to create it as a global function. It shouldn't associate to some class, just because I made it friend of one, should it? Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted May 25, 2007 Report Share Posted May 25, 2007 There is no need for a friend function, this is the wrong idiom. A public static function is equivalent to a C function at file scope, in that it can be called without any class instance, e.g. IMapExpressionPtr expr = IMapExpression::createForToken("heightmap"); It doesn't have to be associated with a class as far as the compiler is concerned, but this is a stylistic device used for tidiness. There is going to need to be a static map associating IMapExpression subclasses with string tokens, and this should be a member of the same class. Essentially, the IMapExpression interface is also functioning as its own factory class. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
mohij Posted May 25, 2007 Author Report Share Posted May 25, 2007 Hey, that is what I tried to do before, but then the compiler told me that it is plainly impossible to call functions from non instantiated classes, now I know better . Cool, learned another thing, thanks. This way it is of course nicer. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 25, 2007 Author Report Share Posted May 25, 2007 I'm away over the weekend, so sadly I won't be able to continue on this before monday. I hope this won't break any timelines. Quote Link to comment Share on other sites More sharing options...
OrbWeaver Posted May 25, 2007 Report Share Posted May 25, 2007 Ha, if there were deadlines on this project I would have had to fire myself several times over for low productivity. Quote DarkRadiant homepage ⋄ DarkRadiant user guide ⋄ OrbWeaver's Dark Ambients ⋄ Blender export scripts Link to comment Share on other sites More sharing options...
mohij Posted May 25, 2007 Author Report Share Posted May 25, 2007 It's quite relieving to hear that, I already felt bad a little bit So till monday then. Quote Link to comment Share on other sites More sharing options...
Crispy Posted May 26, 2007 Report Share Posted May 26, 2007 I, too, would have been unceremoniously thrown onto the street a long long time ago if we had strict deadlines to stick to. Get things done as quickly as you can by all means, but don't stress out if you don't have time. Quote My games | Public Service Announcement: TDM is not set in the Thief universe. The city in which it takes place is not the City from Thief. The player character is not called Garrett. Any person who contradicts these facts will be subjected to disapproving stares. Link to comment Share on other sites More sharing options...
Nyarlathotep Posted May 26, 2007 Report Share Posted May 26, 2007 I'm afraid I'm even worse. I had to drop off the forums for three weeks due to finals and vacationing in Sydney. I've yet to do anything since I've gotten back (apart from playing Twilight Princess). Deadlines are my kryptonite. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 29, 2007 Author Report Share Posted May 29, 2007 I'm back. Quote Link to comment Share on other sites More sharing options...
greebo Posted May 29, 2007 Report Share Posted May 29, 2007 Good to hear that. I didn't follow the discussion above very closely, but maybe a small sketch of the system to be implemented might prove useful here? We did that back when we redesigned the ShaderSystem, and it was good to have some plans around - also for later reference and/or the wiki. Not a must of course, just a suggestion. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 29, 2007 Author Report Share Posted May 29, 2007 Well, what I have since now is: Base class: class IMapExpression { public: static boost::shared_ptr<IMapExpression> createForToken(parser::DefTokeniser& tok); virtual TextureConstructor& getImage() = 0; }; specific Map Expressions then look this way:class HeightMapExpression : IMapExpression { shared_ptr<IMapExpression> argOne; float argTwo; public: HeightMapExpression (parser::DefTokeniser& tok); TextureConstructor& getImage(); }; implementation for createForToken:boost::shared_ptr<IMapExpression> IMapExpression::createForToken(parser::DefTokeniser& tok) { // Switch on the first keyword, to determine what kind of expression this // is. std::string token = boost::algorithm::to_lower_copy(token.nextToken()); if (token == "heightmap") { return shared_ptr<IMapExpression> p(new HeightMapExpression (tok)); } else if (token == "addnormals") { return shared_ptr<IMapExpression> p(new AddNormalsExpression (tok)); } ... I'm not entirely sure where the images should be saved, inside the structures? Also not sure when they should be processed. Either when creating the class or when getImage is called. So, comments please Quote Link to comment Share on other sites More sharing options...
mohij Posted May 29, 2007 Author Report Share Posted May 29, 2007 Forgot, there is also an "ImageExpression" which just returns an image. Edit: is there a way to upload files (.h and .cpp)? That would make things easier, as I could just paste the files I've written so far. Quote Link to comment Share on other sites More sharing options...
greebo Posted May 29, 2007 Report Share Posted May 29, 2007 Yes, we have an FTP server, please read here how to access it: http://forums.thedarkmod.com/index.php?showtopic=609 Do you have some material files for testing? Otherwise I could provide a few to facilitate testing. (Looks good so far, the devil is probably in the detail as usual, but I get the idea.) Quote Link to comment Share on other sites More sharing options...
mohij Posted May 29, 2007 Author Report Share Posted May 29, 2007 What directory would be appropriate? There is a temp and a DarkRadiant dir.I just have the standard doom3 files, no idea if they suffice. Quote Link to comment Share on other sites More sharing options...
greebo Posted May 29, 2007 Report Share Posted May 29, 2007 The DarkRadiant directory will be good enough for this, it's just the snapshot ZIP lying around there, so feel free to drop it there. The standard Doom3 files can be enough I think, just do a grep for addnormals() or heightmap in the material files, I think they are in pak001.pk4 or pak002.pk4. Quote Link to comment Share on other sites More sharing options...
mohij Posted May 29, 2007 Author Report Share Posted May 29, 2007 So here is my current progress:MapExpressionnew.cppMapExpressionnew.h I just looked at the surrounding files and I think I will have to change some other files too to make them work with the new interface, right? Then this should be done first, so I can test the code I write. Should the processed images be saved at all (or just created every time the Shadersystem requests one) and if yes, where?In the Constructor: all MapExpressions would be evaluated, independent of their usagegetImage could also be defined in the base classin getImage: either the MapExpression is recreated everytime it is asked for, or saved (either in the class or pushed to the texture manager) I think in getImage would be better. Different from the files above, I think it would be better to save the results instead of the arguments.Edit: I changed MapExpressionnew.h to reflect this. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.