MapExpressions

[mohij]

Well then, let's proceed to actually implementing the MapExpressions :-)

[mohij]

Would it be a good idea to commit the changes, so the interface is set? That would also make it easier for me, since I could finally update my checkout again.

[greebo]

If you want, you can send me your current state and I will commit it to a separate branch. (You'll have to switch your local copy to that branch then.) I could also go briefly over it and change some things if I see some any serious problems with the code.

[mohij]

Ok, new version is on ftp, same link as before. I didn't change anything except fixing this bug and removing my bug-hunting code. No idea if a seperate svn branch is worth it, but it would be surely helpful, because I wouldn't have to upload those MapExpression.zip files ;-)

[greebo]

Well, you'll still have to send me the files so that I can upload it to that branch. At least for the moment being.

 

I guess, you'll be granted SVN access in the not too distant future anyway, so it probably won't be long, if you keep progressing at that rate.

[greebo]

The new branch is up:

 

https://darkradiant.svn.sourceforge.net/svn.../mapexpressions

 

I will merge in some changes from the trunk, so you might want to keep your local copy updated the next few hours.

[mohij]

OK, here the link (just the shaders folder and ishader.h, as before):

MapExpression.zip

 

I guess it will work with newest head.

[mohij]

A HightMap is defined by the greyscale image and a scale value. Does someone know how this parameter is defined? Ideal case would be a relation to the pixel distance (like 1=from white to black is one pixel distance, 0=means no distance (the image is ignored), 2=two pixeldistances, and so on).

[greebo]

I couldn't find the ishaders.h, but it was just a quick change, so I did that myself.

 

I also merged in some #include changes, as I recently renamed the "preferencesystem.h" to "ipreferencesystem.h".

 

There is a floatToStr() method in string/string.h based on boost::lexical_cast, so I removed the static helper from MapExpression.cpp.

 

Btw: You can create direct links to files on the FTP when using this address (the server IP is different):

http://208.49.149.118/TheDarkMod/directory/filename

 

edit: You may want to have a look at the HeightmapCreator backup file in the shaders/ folder, maybe this proves to be useful?

[mohij]

Where is that piece of code from? I think it will take some time until I fully understand edge detection, as this is the first time I deal with stuff like that.

[greebo]

This code was already in GtkRadiant and it probably comes from the iD software people themselves, as many other snippets in GtkRadiant (the projected light frustum code, for instance).

 

From what it seems, we could try to use it as it is (maybe change its interface) and see what results it gives. If we can't get it to work, we'll have to look into some free algorithms on the web or figure it out on our own.

[mohij]

Well I do roughtly understand what it does (those comment lines, though rare give some hint to what actually happens), but understanding how Edge Detection and in this case the Prewitt filter works is a different thing (my brother said this is stuff for fourth term in university ;-).

 

Hey, I just saw that you already created a new branch, cool, thanks.

[mohij]

I copied the HeightMap thingy over. I put some materials that use the heightmap expression on a brush, but I'm not sure how the result should look like. It is colored and bumped, is that correct?

 

Edit: compared to current head, and that seems to be correct (since current head does not bump)

 

Edit2: am I meant to upload this now? I have no information whatsoever on how to do this.

[greebo]

You can't upload them to SVN currently, so please zip them up to the FTP. I probably won't have time to look at them till the late evening, I'm off to Volleyball right now.

[mohij]

OK, file is online on:

MapExpression.cpp

[greebo]

Yes, the heightmap creator code seems to work. From looking at the code it just seems to approximate derivatives taking the nearest neighbours into account? However, I tested it with a greyscale image and it seems to create a proper normalmap.

 

One step forward! I will commit the file to SVN, although I'll move the heightmap-related code into an include file to keep the MapExpressions file clean.

[greebo]

Ok, the changes are committed.

 

I noticed a memory leak in HeightMapExpression::getImage(), as the heightmap was never released from the heap - only the pointer to the Normalmap was returned by the function. (This would have been automatically resolved by using shared_ptrs for the Images, but this is not the scope of the current refactoring, this is something we might want to do in the future).

[mohij]

I forgot to upload the .h file, sorry for that. But you found the correct values, so I guess it wasn't a problem ;-)

[greebo]

Yes, wasn't too hard to guess.

 

What are you working on next?

[mohij]

I think I found a bug. The "shaderNotFound" texture shows inverted colors on negative normal angles. This has nothing to do with the NormalMap thing, as it already happens before those changes.

 

[greebo]

I noticed that on almost every texture in lighting mode, could it be that the specular map is not delivered as black for shaders with no "specularmap" keyword? Normally a fully black picture should be returned.

[mohij]

I'm not sure how that could happen. Since we fixed the bug with returning white images instead of ShaderNotFound, it is more logical that a ShaderNotFound is returned, when a call is faulty.

The relevant line:

mapExp = IMapExpression::createForString(GlobalRegistry().get("user/paths/bitmapsPath") + IMAGE_BLACK);

[greebo]

How does the createForString() method behave? The argument

GlobalRegistry().get("user/paths/bitmapsPath") + IMAGE_BLACK

evaluates to this: G:/Dark Mod/darkradiant/install/bitmaps/_black.bmp.

 

I assume the MapExpression attempts to load it using the Defaultconstructor? It is very likely that it fails on BMP files, so I think we need a way to tell the MapExpression to load the file directly from the disk (using the FileLoader).

[mohij]

Yeah, this is the second time MapExpressions fail because they use the DefaultConstructor and not the FileLoader.

 

First thing that comes to my mind is overloading the createForToken/createForString functions to take a pseudo argument: "bool useFileLoader=0", but it could be that there is a more elegant way of solving this. But on the other side the question arises if it makes sense to interpret paths as MapExpressions, because in the end a MapExpression was not meant to be a path (that's just a hack to make life easier).

[greebo]

Hm, I think I'd go for the optional argument.

 

It would be too much hassle to equip the MapExpression with a proper path recognition that is able to distinguish between VFS paths and actual paths into the filesystem. There is no consistency with regard to file extensions either, so I believe there is no surefire way to get the interpretation right.

 

The optional argument is ok for now, I reckon. If you can't think of a better solution right now, there is always the possibility to refactor it later on (although that's admittedly not the best approach to coding in the first place, but I can't think of anything better on the short hand).