Not logged inOpenClonk Forum
Up Topic Development / Developer's Corner / Simplify fragment shaders?
1 2 Previous Next
- - By PeterW [gb] Date 2015-09-20 13:01 Edited 2015-09-20 13:04
Following up on a discussion in #openclonk-dev... The idea with the "slice" model was that we could easily "plug" shader components into each other. So - for example - somebody could override the "scaler" shader to get a different landscape scaling behaviour without the need to override other parts of the shader.

However, I'm not quite sure this is going to happen, because clearly the whole thing is quite hard to understand currently. From having a look through the source code, we currently have the following shader "groups":
* UtilShader, LightShader, AmbientShader, GammaShader: Always applied.
* LandscapeShader, ScalerShader: Applied to the landscape.
* ObjectBaseShader, ObjectLightShader: Applied to meshes + sprites. This also applies to GUI elements, from what I can see.
* SpriteTextureShader: Applied to meshes ("unused"?) + sprites when requested. Not entirely sure what this does.
* SpriteOverlayShader: Applied to meshes + sprites when requested. Probably where player colour comes in?
* MeshShader: Gets loaded, but doesn't even exist?
* Shaders dynamically generated from Ogre files.

Furthermore, we have the following preprocessor macros switching on and off certain parts of the shaders:
* LANDSCAPE: Set for landscape drawing
* MESH: Set for meshes
* HAVE_LIGHT: Assume we have light information. If set, LightShader+AmbientShader query their respective textures to find how much light we have at the part of the landscape in question. Otherwise, we fall back to a basic shading model that makes sense for showing 3D GUI items.
* HAVE_NORMALMAP: We have a normal map for our material, use it appropriately for shading calculations.
* CLRMOD_MOD2: Adds player colour overlay instead of multiplying it.
* HAVE_2PX: Set by the landscape scaler, because we need to shade two full pixels for anti-aliasing at the edges.

All in all, this is a rather dizzying number of possible configurations, a lot of which aren't even used. Some things are turned on and off by adding slices, others by preprocessor macros. So how about we do the following:
1. Merge UtilShader+LightShader+AmbientShader+GammaShader into CommonShader
2. Merge LandscapeShader+ScalerShader as LandscapeShader
3. Merge ObjectBaseShader+ObjectLightShader+SpriteTextureShader+SpriteOverlayShader as ObjectShader (?)

Furthermore, we map *all* C4SSC shader flags (C4SSC_MOD2, C4SSC_BASE, C4SSC_OVERLAY, C4SSC_LIGHT, C4SSC_NORMAL) to shader preprocessor flags. The landscape shader will set them too for consistency. LANDSCAPE, MESH & SPRITE (?) set the drawing mode. End result: Everything in 3 files, we can focus on writing some good documentation for the preprocessor flags, hopefully overall less mess.

Makes sense? Clonk-Karl, especially?
Parent - - By Clonkonaut Date 2015-09-20 13:31
There's also a preprocessor macro to sort out the SKY!
Reply
Parent - - By PeterW [gb] Date 2015-09-20 14:02
You mean there should be? Can't find anything.
Parent - - By Sven2 [us] Date 2015-09-20 14:12
It has been added recently. Maybe you need to update.
Parent - - By PeterW [gb] Date 2015-09-20 14:25
Ah, yes, the "rimlight" change. I'll probably re-implement that one anyway.
Parent - - By Sven2 [us] Date 2015-09-20 14:26
Will you replace rimlight by simple ambience then? The rimlight looked rather weird on many objects.
Parent - - By PeterW [gb] Date 2015-09-20 14:40
Something like that, yes. A bit confused by the whole situation, to be honest - we were using ambient shading before that already, right? Also, why does that commit change it so the sky illumination is coming from the side?
Parent - - By Clonkonaut Date 2015-09-20 14:43
Because sun!!
Reply
Parent - - By PeterW [gb] Date 2015-09-20 14:51
So the Clonk sun is always on the right side of the landscape?
Parent - By Clonkonaut Date 2015-09-20 15:01
Until we can change that by using SetShaderVariable() or whatever ck had in mind: Yes. :D
Reply
Parent - - By Clonkonaut Date 2015-09-20 14:32
I didn't do the macro, so it can't be the rimlight change (and it isn't).

Before that change, you couldn't distinguish between sky and sprites.
Reply
Parent - By PeterW [gb] Date 2015-09-20 14:42
Hm, okay, sorry, sloppy research.
Parent - - By PeterW [gb] Date 2015-09-20 15:46 Edited 2015-09-20 15:48
Proposal in repository ("lights3"), together with the "shiny materials" stuff I was working on. Merge seems tricky, I'd rather wait until we know exactly what we want to do.

Btw - doesn't 790219ac kill transitions for material animations? Not the most critical use case, granted...
Parent - - By Zapper [de] Date 2015-09-20 15:55
Do you have screenshots? :)

PS: okay, that was stupid of me. You didn't do anything about the rimlighting/ambient lighting but just the cleanup stuff, yet, right? :D
Parent - - By PeterW [gb] Date 2015-09-20 17:04 Edited 2015-09-20 17:12
I converted some light to use ambient shading (50/50), which - as far as I'm concerned - would be the better solution. Actually surprised that ck's existing code didn't already achieve a satisfactory effect. However, all light levels are probably off now, so it will be hard to directly compare.
Parent - By Zapper [de] Date 2015-09-20 17:37
Rimlighting had the effect that the objects didn't look as flat as with simple ambient lighting, though. For some models that was really an improvement (kitchen f.e.) while for others it was debatable. But I still liked the change in general.
Parent - - By Clonk-Karl [us] Date 2015-09-20 15:59

> Btw - doesn't 790219ac kill transitions for material animations? Not the most critical use case, granted...


Does it? If yes, how so? I thought this to be a pretty much 1:1 translation of the 3D texture to the 2D array.
Reply
Parent - - By PeterW [gb] Date 2015-09-20 17:02
I think a 2D array will round the index? The 3D texture doesn't do that, and that's used for interpolating between material animation frames.
Parent - By Clonk-Karl [us] Date 2015-09-20 17:30
Oh, interesting. I was confused by the float-to-int-and-back-again conversions in queryMatMap, so I thought this is precisely to prevent any interpolation and make sure the correct "slice" of the 3D texture is sampled. With this in mind I still think the texture array is a better option, and transition could be achieved by sampling twice from it. But at the moment with the current textures it does not make much of a difference anyway.
Reply
Parent - - By Clonk-Karl [us] Date 2015-09-20 16:09

> normal = normalize(normal);


Is that really needed in CommonShader.glsl? Shouldn't the part of the code that is providing the "normal" variable make sure it's normalized?
Reply
Parent - By PeterW [gb] Date 2015-09-20 17:03
True. There was some code that didn't adhere to that (probably mine) so I added that so I don't end up making the same mistake again. Should be removed once the shader's finished.
Parent - - By Clonk-Karl [us] Date 2015-09-20 15:55 Edited 2015-09-20 16:03
The current situation basically originates from my changing the full rendering of everything to use shaders, which was necessary as part of the FoW work so that sprites, meshes and other things are properly covered by FoW. Of course I experimented a bit with different possibilities and now we have something that works but yes, is somewhat messy. It certainly doesn't help that we have so many different configurations. I think it's a worthwhile effort to try and clean it up a bit. Let me first correct/clarify some of your observations:

> * SpriteTextureShader: Applied to meshes ("unused"?) + sprites when requested. Not entirely sure what this does.


This does not apply to meshes but only for sprites. It multiplies the base color of the sprite that is set with glColor*() with the sprite's texture. Note that the "sprite" shaders are also used for other 2D drawing such as points and lines, such as PXS. Those don't have a texture and so the shader for those doesn't include SpriteTextureShader.glsl.

> * SpriteOverlayShader: Applied to meshes + sprites when requested. Probably where player colour comes in?


Again, only applies to sprites. Modulates the pixel color with the color from the overlay texture (Overlay.png) -- so yes, primarily, player color.

> * MeshShader: Gets loaded, but doesn't even exist?


Whoops. I probably started with MeshShader.glsl but then changed my mine at some point. Can go away of course.

> * Shaders dynamically generated from Ogre files.


These can either be dynamically generated or hand-written, e.g. when enabling normal maps for meshes a hand-written shader must be used, which is in Objects.ocd/normal_map_fragment.glsl (not a good location really, but the only place that makes sure this gets loaded before other OGRE materials, so it can be used by other OGRE materials. We might want to load OGRE materials from Graphics.ocg, too, so we can move it there).

> * LANDSCAPE: Set for landscape drawing


Now OC_LANDSCAPE.

> * MESH: Set for meshes


Now OC_MESH.

> * HAVE_LIGHT: Assume we have light information. If set, LightShader+AmbientShader query their respective textures to find how much light we have at the part of the landscape in question. Otherwise, we fall back to a basic shading model that makes sense for showing 3D GUI items.


Now OC_DYNAMIC_LIGHT. This is usually set for ingame objects and not set for GUI objects. Not only relevant for 3D but also 2D sprites.

> * HAVE_NORMALMAP: We have a normal map for our material, use it appropriately for shading calculations.


Now OC_WITH_NORMALMAP. Used both for sprites and meshes; for meshes the shader slice in Objects.ocd/normal_map_fragment.glsl activates this so it is picked up by ObjectLightShader.glsl.

> * CLRMOD_MOD2: Adds player colour overlay instead of multiplying it.


Now OC_CLRMOD_MOD2. Doesn't affect player color but color modulation as set with SetClrModulation. This is activated when the blit mode is set to GFX_BLIT_Mod2 with SetObjectBlitMode, and also used for highlighting objects in the editor when pressing Shift.

> * HAVE_2PX: Set by the landscape scaler, because we need to shade two full pixels for anti-aliasing at the edges.


Now OC_HAVE_2PX.

Plus we now have OC_SKY as mentioned by Clonkonaut.

Overall I think your suggestion makes sense, but we still need all these preprocessor flags, right?

> 3. Merge ObjectBaseShader+ObjectLightShader+SpriteTextureShader+SpriteOverlayShader as ObjectShader (?)


This will need additional preprocessor flags/checks to distinguish meshes from sprites with texture from sprites without texture from sprites with texture and overlay. But that should be fine. Overall I'm not sure we gain too much, but it probably makes sense to try it out and see how it works. I'm somewhat worried we merge some of the files but then have more preprocessor checks in those files. I don't know what's better.
Reply
Parent - - By PeterW [gb] Date 2015-09-20 17:11

> This does not apply to meshes but only for sprites. It multiplies the base color of the sprite that is set with glColor*() with the sprite's texture. Note that the "sprite" shaders are also used for other 2D drawing such as points and lines, such as PXS. Those don't have a texture and so the shader for those doesn't include SpriteTextureShader.glsl.


I guess the part that confuses me is - why do we need a special path for that? To do the color multiplication?

> ... renamings ...


Yeah, saw that you changed it. I would have preferred to keep the naming closer to the C4SSC names, but well.

> This will need additional preprocessor flags/checks to distinguish meshes from sprites with texture from sprites without texture from sprites with texture and overlay.


Beyond the OC_WITH_BASE / OC_WITH_OVERLAY that I added in lights3?
Parent - By Clonk-Karl [us] Date 2015-09-20 17:33

> I guess the part that confuses me is - why do we need a special path for that? To do the color multiplication?


Because in some cases we don't have a texture, so what do you want to multiply it with? Do you mean we could instead bind a 1x1 texture with a single (1.0, 1.0, 1.0, 1.0) pixel?

> Yeah, saw that you changed it. I would have preferred to keep the naming closer to the C4SSC names, but well.


I mostly changed it to add the OC_ prefix, but didn't think too much about the rest of the name. So since you are cleaning these things up anyway, feel free to change the definitions to match the C4SSC names.

> Beyond the OC_WITH_BASE / OC_WITH_OVERLAY that I added in lights3?


No, that should be enough. I wrote my reply before I saw your commits :)
Reply
Parent - - By PeterW [gb] Date 2015-10-03 15:57 Edited 2015-10-04 14:44
Okay, this is now in the repository. I especially rebalanced shading a bit to go more into the direction of "rimlighting". Here's before and after:





Will probably still need a few iterations to look good in all situations... Especially note that the clouds in the second screen shot are too bright now.
Parent - - By Marky [de] Date 2015-10-03 16:05
Is the before-version somewhere, too? I want to use it for a specific scenario
Parent - - By PeterW [gb] Date 2015-10-03 16:11
You can always override the shaders per scenario. Might not be entirely trivial to adapt though, given that I have restructured the shader files quite a bit. Hm.
Parent - By Marky [de] Date 2015-10-03 16:13
Yes, I just want to paste your old setting into my scenario, if you still have it.
Parent - - By Clonk-Karl [us] Date 2015-10-03 16:36 Edited 2015-10-03 16:48
Cool. I like how everything is much brighter and more saturated now. It seems to me that in some scenarios, e.g. Crash.ocs, the sky is too bright now, though. You also changed the behaviour of ambientBrightness, probably not intentionally(?) (#1413).

Edit: Also the GUI elements are fairly dark now, e.g. the Clonk picture in the HUD is too dark compared to the Clonk in-game. Selections in editor mode are now gray where they used to be white before.
Reply
Parent - - By PeterW [gb] Date 2015-10-03 17:32
Hm. It's intentional insofar that the old behavior didn't make sense to me. Why would we ever want the sky to be illuminated by the Clonk's light? On the other hand, you have a point about objects and landscape.

> Edit: Also the GUI elements are fairly dark now


Yeah - but if I change this, the main menu screen will become overly bright.
Parent - - By Clonk-Karl [us] Date 2015-10-03 17:41

> Why would we ever want the sky to be illuminated by the Clonk's light?


At nighttime? I think this scenario of Sven does this.

> Yeah - but if I change this, the main menu screen will become overly bright.


But it should not be that a call to C4Draw::DrawLineDw() with color white gives you a gray line. Maybe the main menu should use a different shader?
Reply
Parent - By PeterW [gb] Date 2015-10-04 14:59
That now works exactly like I'd imagine it - the sky remains there no matter the light, objects become black silhouettes against it. Do we already have a flag to keep stars from disappearing?
Parent - By Sven2 [us] Date 2015-10-03 17:51
I think for the night sky, some illumination is desired. For daylight sky it's typically not. Would it be possible to have a parameter for that?

For daylight, e.g. see the fireflies in Crash Landing. They look a bit stupid on top of that hill during the day.
Parent - - By Clonk-Karl [us] Date 2015-10-03 20:14
Just for the record because it's probably being related to other things being dark as well: global viewports, including "New Viewport" in editor mode and full map screenshots, probably also spectator viewports in network games, are much darker than player viewports.
Reply
Parent - - By Pyrit Date 2015-10-03 23:23
Sprites are kind of dark, too.
https://trello.com/c/M7OKF7jC/96-lighting

This was before Peters change, tough.
Parent - By PeterW [gb] Date 2015-10-04 14:46
Can't access that, but hopefully the new version should iron out most of the kinks.
Parent - - By PeterW [gb] Date 2015-10-04 14:39 Edited 2015-10-04 14:46
New version pushed. Actually went back on a lot of changes - ambient light is probably a bit less bright now, but on the other hand now textures facing the viewer appear "exactly as specified" when illuminated by ambient light. This is probably the morally right thing to do. I also took over the "right-side ambient light" from rim-lighting, as after some experiments I agree that it looks more dynamic.

Also made an effort to pull out all the important constants into the "init" slices of CommonShader and LandscapeShader. Furthermore, all shaders now automatically refresh when the underlying files are changed. This means that it should be a lot "safer" to experiment now, can only encourage everybody to try it!

Edit: See post above for new comparison shots.
Parent - - By Pyrit Date 2015-10-04 15:53
Could it be that it broke FoW for transparent materials? Instead of black you see sky now. (Yesterday it still worked)
Parent - - By PeterW [gb] Date 2015-10-04 16:21 Edited 2015-10-04 16:24
Well, see above, Sky is always visible now. I suppose we want material to become less transparent when inside the FoW?
Parent - - By PeterW [gb] Date 2015-10-04 17:03
I guess we could also approach it the other way around: Shouldn't transparent materials cause some amount of ambient lighting?
Parent - - By Pyrit Date 2015-10-04 18:03
Yes, that would be logical.
Either have the shader itself figure out how transparent the texture of the material is, or
maybe have an extra parameter in *.ocm to determine how much ambience shines through? :F
Parent - - By PeterW [gb] Date 2015-10-04 20:10
The latter would make sense, and is probably quite easy to implement. Any objections? How commonly are semi-transparent materials used anyway?
Parent - - By Marky [de] Date 2015-10-04 21:45
I want to use them in at least one scenario :p How do semi-transparent materials with a background material behave?
Parent - - By Clonk-Karl [us] Date 2015-10-04 22:09
Background materials don't affect drawing whatsoever.
Reply
Parent - - By Marky [de] Date 2015-10-05 18:00
Semi-transparent water with a brick tunnel background would have been cool!
Parent - - By Clonk-Karl [us] Date 2015-10-05 23:41
It would be fairly expensive computationally to actually draw the background material, and it would only have little benefit. You could (partly?) emulate such behaviour by drawing the brick onto the sky, or by having a brick decoration object that's being drawn behind the landscape.
Reply
Parent - - By PeterW [gb] Date 2015-10-06 09:43 Edited 2015-10-06 09:50
We *could* change Sky drawing to use a shader that switches to the background material. Not sure I like that solution very much either though, given that it would have to do scaling *and* light calculations too.

Edit: Heh, if tiling matches, a dirt-cheap way of making this happen would be to have the engine automatically produce a "water with X background" material texture. Bonus points if it works with animation. Getting normals right might be tricky though.
Parent - - By PeterW [gb] Date 2015-10-06 18:24
Hm, we have a second texture for background? That makes it harder...
Parent - - By Clonk-Karl [us] Date 2015-10-06 19:52
We don't have a texture at all for it. Just an 8-bit surface in main memory.
Reply
Parent - By PeterW [gb] Date 2015-10-07 08:39
Hm, could still try to integrate that data into the landscape surface, by dynamically allocating foreground-background pairs and adding additional textures when needed. Still tricky.
Up Topic Development / Developer's Corner / Simplify fragment shaders?
1 2 Previous Next

Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill