Source Code Project Mantis - FSSCP
View Issue Details
0001914FSSCPOpenGLpublic2009-04-11 07:182009-11-19 08:29
ReporterKeldorKatarn 
Assigned ToHery 
PriorityhighSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.6.11 
Target Version3.6.12 RC1Fixed in Version3.6.12 RC1 
Summary0001914: Skybox rendering uses texture GL_REPEAT instead of GL_CLAMP causing seams to appear.
DescriptionWe encountered this in Saga. We recently introduced self made skyboxes instead the starfield-skysphere + FRED nebulas. We noticed visible seams between the 6 skybox textures. The reason seems to be the usage of GL_REPEAT in conjunture with small fractional float errors in the texture coordinates.
We could correct for this in the shaders by simply simulating GL_CLAMP in them, but that is no solution for the fallback fixed function pipeline rendering.

Since it is unlikely that tiling will be used by anyone for skyboxes, it would make sense to introduce CLAMP to avoid this issue. A test would be needed though if setting to CLAMP will be enough. I don't know if overflow by rounding error can still occur then and what that would look like. Also one has to play great attention ingame when testing. Depending on what skybox is used one might not spot this problem. if only black backrgound and stars are visible one won't see an overflow from black to black. but with planets nebulas or even a landscape skybox this will be visible to some degree or even very obvious depending on the skybox color layout.
TagsNo tags attached.
Attached Filespatch skybox_rendering.patch (739) 2009-05-11 15:21
http://scp.indiegames.us/mantis/file_download.php?file_id=1271&type=bug
patch skybox_clamp.patch (5,263) 2009-11-14 08:12
http://scp.indiegames.us/mantis/file_download.php?file_id=1347&type=bug

Notes
(0010882)
KeldorKatarn   
2009-05-11 15:22   
Since nobody seems to ...
Uploaded a patch.
(0010883)
taylor   
2009-05-11 20:11   
The gr_opengl_* functions should never be referenced outside of the OpenGL code without very good reason. Just use the standard function pointer instead, that's what it's there for.

I still don't like this as a real fix though. While skyboxes may not make use REPEAT, skyspheres might. We have no way to know which is in use so we can't make use of both, just one or the other completely. There is a possibility that this could end up breaking another mod.

I haven't really kept up with the problem either, but I have seen/used about a dozen skyboxes over the years and have never had a single issue with the tex coords. I find it difficult to believe that this is just a code issue, though I suppose it is possible (just highly unlikely).
(0010884)
Tolwyn   
2009-05-12 06:28   
I doubt it is somehow related to the texture coordinates - DaBrain checked them as well as I did.

http://www.hard-light.net/forums/index.php/topic,62783.0.html
(0010885)
taylor   
2009-05-12 16:06   
I also doubt that it's a simple texcoord issue. But the fact remains that it has worked fine historically, and is only recently having issues. Whether there is now a rendering issue (possibly shader related, anybody tested with -no_glsl?), a POF conversion/reading issue due to newer tools, of if older skyboxes actually had some strange hack which overcame the issue, I have no idea. My point is just that we can't simply treat this as an issue which is completely broken, because it did and still does work (last time I was playing with SoL at least).

I'm going to go back over some older content and verify whether it still really works 100% or not, and whether there is a difference between shader/fixed rendering. If you can attach a POF and maps (doesn't have to be official content) which are known to have the problem then that would help as well.
(0010886)
Tolwyn   
2009-05-12 16:36   
I could upload it later - I am using sky5.pof I got from SoL
(0010891)
Tolwyn   
2009-05-13 02:39   
Allright, let's get to it. ;)

This issue does occur with and without glsl. Actually, the original hack-ish solution was to counteract this in the shader. It worked, to some extent (for example seems were still visible during autopilot transitions, which use different FOV due to cutscene bars). With Keldor's latest attempt, however, seams are gone on all systems. It does not matter if shader support is enabled or not.

Here is a picture taken on Radeon X1250, which has no shader support

http://i278.photobucket.com/albums/kk103/limdaepl/loki3test1.jpg

The skybox POF used originates from SoL, as I mentioned in my previous note. I compiled a package containing one of our skyboxes plus a set of texture maps and will send you a PM with the download link.

Thanks for looking into this issue. It is very appreciated. ;)
(0010892)
taylor   
2009-05-13 04:01   
Tweaking the UVs appears to fix the seams, visually at least. Basically it's just cheating the edge blending and isn't the best solution, but it keeps the fix in the content which is less error prone than a code fix against an antiquated texture setup. This is another one of those things that would be obscenely simple to deal with if we had a material system. :(

I just made the changes in code, so a proper test is needed to see if there is really enough of a difference to qualify as something that will work. See if Scooby (or whoever can do it) can modify the tex coords in the POF so that 0.0 is changed to 0.0005. It's a little backwards from how I thought that might resolve things, but it looks to work fine in my few quick tests. Oh, and you may need to delete the IBX file to make sure that it uses the new values rather than the old ones by mistake.
(0010893)
Tolwyn   
2009-05-13 05:43   
OK, I am on it.

So all you need is a skybox with 0.0005 - 0.9995 UV coordinates?
(0010894)
taylor   
2009-05-13 05:49   
I tested with 0.0005 - 1.0. Not sure if it makes that much difference. We'll see what Scooby thinks.
(0010896)
Tolwyn   
2009-05-13 06:55   
I am not that comfortable with the solution - it sounds like a hack, to tell the truth.
(0010898)
taylor   
2009-05-13 11:18   
It's a long accepted way to get rid of skybox seams actually, you can google it if you want. Using clamp-to-edge is the other of the two possible methods to address the problem, but with our code there is simply no way that can be used properly without breaking something else.
(0010909)
karajorma   
2009-05-18 18:25   
Why not simply add a property to the mission file to say whether or not to use CLAMP or REPEAT?

Something like Bobb's skybox flags which I made the FRED changes for recently.

http://svn.icculus.org/fs2open?view=rev&revision=5080
(0010984)
KeldorKatarn   
2009-06-20 16:06   
Would work for me...
(0010986)
Aardwolf   
2009-06-23 20:14   
Doesn't the mediavps skybox (i.e. starfield.pof) use tiled textures, and thus require GL_REPEAT ?
(0010990)
karajorma   
2009-06-24 02:39   
That's why I'd like to make this a flag you can set in FRED. It's allowing the mission designer a lot lower level access to the render code than I'd like but it's probably the best way of deciding unless we want to make it a pof feature (Which I suspect is better but would require changes to PCS II as well as the code IIRC).
(0010991)
KeldorKatarn   
2009-06-24 12:17   
In the long run it would be best to make this a POF per-texture-setting feature, yes. But since this is obviously way beyond the scope of this problem right now, I'd say let's go with the flag for now, which can later be easily removed again once it is decided to implement it into POF on a larger scale.
(0010996)
KeldorKatarn   
2009-06-26 13:00   
-This is a Saga showstopper-
(0011117)
chief1983   
2009-07-29 16:42   
Is there a way for the code to know whether the texture is tiled or not and use the appropriate method? Then it wouldn't require any POF or mission changes.
(0011118)
Tolwyn   
2009-07-29 16:48   
I don't think so. Then again, one could probably tie this to -wcsaga flag. It'd be a dirty solution, but an easy one.
(0011119)
chief1983   
2009-07-30 12:55   
There was worry that changing the behavior could have negative side effects, but it sounds like anyone using a UV'd skybox would definitely want CLAMP, and anyone using tiling would definitely want REPEAT. Could this behavior be set based on the polymodel flag PM_FLAG_ALLOW_TILING? Oddly I can't find where in the code that actually gets set though, but it does get checked in a couple of places in modelinterp.cpp.
(0011233)
chief1983   
2009-11-09 22:35   
I'm throwing this Hery's way cause I don't think he's seen his share of bugs yet :)

Really though, if this isn't your thing just send it back and I'll find someone who can. Catch me in IRC if you do think you can handle it but have some questions or something.
(0011245)
Hery   
2009-11-11 08:20   
(Last edited: 2009-11-11 08:30)
Unfortunately, PM_FLAG_ALLOW_TILING is not an instant solution to this problem. I couldn't find any model in mediavp that sets this flag (even if it wants REPEAT). Hence, if PM_FLAG_ALLOW_TILING determined texture mapping behaviour, we would completely break compatibility (probably buggy though).

Anyway, it is model that has to tell the engine if it wants REPEAT or CLAMP. That's why I think the best way to solve this problem is to add new flag PM_FLAG_FORCE_CLAMP to the POF files. I'm not very familiar with the modeling and pof files, so I would be glad if someone told me whether such solution (i.e. adding new flag) has any serious drawback from the modders point of view.

There might be also another possibility. The engine may check (basing on the texture coordinates) whether CLAMP or REPEAT is needed. However, currently I'm not sure if it will be possible.

(0011279)
Hery   
2009-11-14 08:14   
I've added new flag in FRED "Force Clamp" that forces skybox texture to be clamped instead of repeated. It is possible to use this texture also in other models.
Debug build is available here:
http://www.quarnos.org/dl/skybox_build.zip
Please, confirm that the problem is solved.
(0011280)
Tolwyn   
2009-11-14 08:33   
Sounds like a good solution. I'll ask Keldor to disable clamp in our shader set - personally I have no idea where to start looking... ;)
(0011307)
Hery   
2009-11-19 08:29   
chief1983 committed my patch to the trunk.

Issue History
2009-04-11 07:18KeldorKatarnNew Issue
2009-05-11 15:21KeldorKatarnFile Added: skybox_rendering.patch
2009-05-11 15:22KeldorKatarnNote Added: 0010882
2009-05-11 20:11taylorNote Added: 0010883
2009-05-12 06:28TolwynNote Added: 0010884
2009-05-12 16:06taylorNote Added: 0010885
2009-05-12 16:36TolwynNote Added: 0010886
2009-05-13 02:39TolwynNote Added: 0010891
2009-05-13 04:01taylorNote Added: 0010892
2009-05-13 05:43TolwynNote Added: 0010893
2009-05-13 05:49taylorNote Added: 0010894
2009-05-13 06:55TolwynNote Added: 0010896
2009-05-13 11:18taylorNote Added: 0010898
2009-05-18 18:25karajormaNote Added: 0010909
2009-06-20 16:06KeldorKatarnNote Added: 0010984
2009-06-23 20:14AardwolfNote Added: 0010986
2009-06-24 02:39karajormaNote Added: 0010990
2009-06-24 12:17KeldorKatarnNote Added: 0010991
2009-06-26 13:00KeldorKatarnNote Added: 0010996
2009-07-29 16:42chief1983Note Added: 0011117
2009-07-29 16:48TolwynNote Added: 0011118
2009-07-30 12:55chief1983Note Added: 0011119
2009-11-09 22:35chief1983Note Added: 0011233
2009-11-09 22:35chief1983Assigned To => Hery
2009-11-09 22:35chief1983Prioritynormal => high
2009-11-09 22:35chief1983Statusnew => assigned
2009-11-09 22:35chief1983Product Version3.6.9 => 3.6.11
2009-11-09 22:35chief1983Target Version => 3.6.12 RC1
2009-11-11 08:20HeryNote Added: 0011245
2009-11-11 08:30HeryNote Edited: 0011245
2009-11-14 08:12HeryFile Added: skybox_clamp.patch
2009-11-14 08:14HeryNote Added: 0011279
2009-11-14 08:33TolwynNote Added: 0011280
2009-11-19 08:29HeryNote Added: 0011307
2009-11-19 08:29HeryStatusassigned => resolved
2009-11-19 08:29HeryFixed in Version => 3.6.12 RC1
2009-11-19 08:29HeryResolutionopen => fixed