View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002078 | FSSCP | graphics | public | 2009-12-23 19:00 | 2010-12-09 22:41 |
| Reporter | KeldorKatarn | Assigned To | Hery | ||
| Priority | normal | Severity | tweak | Reproducibility | always |
| Status | closed | Resolution | suspended | ||
| Platform | IBM PC | OS | MS Windows | OS Version | Vista SP2 |
| Product Version | 3.6.11 | ||||
| Summary | 0002078: Bloom shader: Bright-Pass Shader input texture has the wrong size | ||||
| Description | The bright-pass shader gets the screen image as an imput texture and renders to a texture a quarter its size. This doesn't work well since this way pixels of the original screen image are lost. Bright points which are only a few pixels wide will flicker when the camera is moved, since the sampling sometimes hits them, sometimes not. It would be better to give the shader the screen image already downsized with bilinear filtering enabled. That way the shader doesn't miss bright spots and the flickering vanishes. Of course the shader could do the bilinear sampling too, but I think the other way would be faster and also the bright-pass filter is passed no information about the screen texture's size at the moment, and from what I know texture size can only be accessed by built in functions since Shader Model 4.0. | ||||
| Tags | No tags attached. | ||||
|
|
There is no "wrong size" of input texture here. The problem is that bilinear filtering works best when downscaling texture to 1/2 its orignal size. Currently, FSO blurs textures using fixed kernel and different sizes of input textures. That make it necessary to blur texture 4 time smaller than the orignal scene image in order to get satisfactory results. The best way to make flickering less visible is to create bloom from few different size textures. Each texture two times smaller than the previous one. (That will make bilinear filtering happy.) That's already done, but I need to test a few things before I post the build here. The problem is not in the bright pass shader code which accepts textures of all sizes, but in the FSO code. |
|
|
Nonono, you missunderstood me. The problem is, that right now the bright pass filter renders from a large texture to a small texture. That way the shader misses pixels and it depends on the movements of the camera whether a small bright object like a star is sampled or not. This results in flickering. If you scaled the input image down in the code, using bilinear interpolation to preserve the information while scaling down, the bright pass shader would work on equally sized texture for input and output and no pixels would be lost anymore, and no more flickering. Edit: I already tried this by hardcoding bilinear-interpolation with a hardcoded screen resolution directly into the bright pass shader and it worked great but was of course way too slow. After all you need 16 samples per pixel for a downscaling of factor 4. So this needs to be done by OpenGL in the engine. As long as this is not fixed bloom will never work without visible flickering whenever small bright spots are on the screen, no matter what you do with blurring. If there is nothing on the output of the bright-pass shader, then there is nothing to blurr. You cannot repair it that way. The information needs to pass through the bright pass shader. Right now 15/16 of the image information is lost! Right now you skip 15 out of 16 pixels. |
|
|
OK, I got it. Actually, all these changes I described previously are also needed since 16 samples per pixel on a full screen texture is way too much no matter when and where the downsampling is performed. I've already found a few methods to do this quite quickly, so I will need to experiment with these various solutions and find the best one. (I'm afraid that "standard" one-pass bilinear downsampling may be still to slow when the screen resolution is big). The graphic effects have to look good and not necessarily be accurate :P If I manage to eliminate that flickering without 4 samples per pixel it also will be a good solution. |
|
|
This has nothing to do with accuracy. It's about quality. You CAN'T get rid of the flickering and at the same time keep the bloom effective without sampling the entire image. And I told you bilinear downsampling was too slow in the shader already, that's why I suggested letting the GPU handle it with built in stuff, which should be accessable via OpenGL from within the engine. The shader shouldn't have to worry about that. Also downsampling to 1/4 image size is 16 samples per pixel, not 4 samples per pixels. And I guarantee you that you can't get rid of the flickering without sampling the entire image or sample a downsampled image. If you drop pixels you will always have flickering because the pixel dropping is exactly what's causing this. You need to preserve the image information or bloom will never look good. |
|
|
All textures used in post-processing are upsampled and downsampled using bilinear filtering. Bilinear filtering is performed when bright-pass shader samples input texture, no additional work is needed. Bright pass shader would skip pixels if GL_NEAREST minifying function was used. That means that the only necessary change is what I described in my first post. OpenGL bilinear filtering uses always 4 samples, what means that it is usable only for downsampling texture 1/2 its previous size. That's why currently flickering appears since the bloom code downsamples texture to 1/4 its previous size. |
|
|
Patch committed. |
|
|
What did you change? Now the shader's seem to have no effect anymore. No bloom visible with the old shaders even on 160 intensity. it would REALLY help if you'd document this feature in detail. You keep changing the code without saying what is different now. I still have no idea what texture is scaled to what and fed how many times into what shader. if you want people to use this and write their own bloom shaders then you need to explain what this does and how it does it, in detail. Right now it doesn't seem to do anything at all. |
|
|
Commit message for that patch "two pass bilinear downscaling in order to reduce artifacts". That's what I changed. Bright-pass shader gets full size texture and "returns" 1/2 of its original size. Then, blur shader gets that downscaled texture and returns 1/2 of its original size (1/4 of the original scene image size). However, I don't think that more documentation than commit message + updated code comments is needed. This change affects only FSO source code and anyone interested can easily figure what's changed in this patch. The problem with disappearing bloom is a shader issue, however I'm going to make FSO invulnerable for that problem in shaders. |
|
|
All I know is, that before the shaders worked and now they don't. So something about the engine or what it feeds into the shaders has changed. So you broke compatibility at one point and it would be nice to be informed as to what that is. Furthermore the entire bloom effect still doesn't do what it should do. I gave you the articale and tried to explain it, but ok. |
|
|
The problem was a bit in a different place I thought earlier and that's why I took me more time to fix it. Now, old shaders should work. I know that the bloom doesn't work how you wanted it to. This report was about flickering and that's solved. |
|
|
True, but the real issue I'm having with this, is that although you know that is isn't working as intended yet, this keeps going into trunk. From my point of view this is still totally experimental code. You keep mentioning the intention to change stuff and Bloom also isn't controlled by the post-processing table yet, but by the command line. That all looks like experimental, non-final code. As long as that is the case this shouldn't go into trunk but into a separate working-on-in-branch. stuff in trunk is used by modders and expected to work. If we write shaders for stuff and later you change something that breaks backwards compatibility because suddely the functionality does something different, that doesn't exactly help and forces us to rewrite our shaders. I really like that this is being worked on, but I'd vote for keeping stuff that's still subject to changes out of the trunk. Also... Goober just opened a thread in which he asks for code to be TESTED before it goes into trunk. You have a habit of committing stuff that breaks builds or has some other small bugs in it. I love that you're working on this, and I really appreciate that you give a lot of feedback on the stuff, but if you commit something it must be tested and work. Projects work with nightly builds and depend on stuff being at least tested to some degree. |
|
|
It is working as intendend now. This is the behaviour (unless some new bugs will be found) that I expect to see in the next release. That's no longer experimental. I've been mentioning further changes but it's a long term plan and is not going to happen soon. That patch didn't broke the backwards compatibility. There were two bugs and as said in SCP policies there were too patches. Unfortunately, the second one was a bit delayed. And about my patches and broken builds. I don't want to discuss it heare since it's not a good place for such thing. I admit that post-processing broke few things but I haven't seen too much people testing Antipodes 4. My other patches, sometimes solved problem only partially, but that can't be considered as a broken build. Could you confirm if this particular problem with flickering is solved? |
|
|
I'm working on university stuff atm, but I'll test this with our shaders as soon as I have time. |
|
|
Closing, due to graphics pipeline rewrite requiring this to be restarted from scratch (if it is still an issue) |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2009-12-23 19:00 | KeldorKatarn | New Issue | |
| 2009-12-23 21:53 | Hery | Status | new => assigned |
| 2009-12-23 21:53 | Hery | Assigned To | => Hery |
| 2009-12-23 22:00 | Hery | Note Added: 0011462 | |
| 2009-12-23 22:02 | Hery | Note Edited: 0011462 | |
| 2009-12-23 23:31 | KeldorKatarn | Note Added: 0011463 | |
| 2009-12-23 23:45 | KeldorKatarn | Note Edited: 0011463 | |
| 2009-12-23 23:47 | KeldorKatarn | Note Edited: 0011463 | |
| 2009-12-24 12:04 | Hery | Note Added: 0011466 | |
| 2009-12-24 13:21 | KeldorKatarn | Note Added: 0011467 | |
| 2009-12-24 22:10 | Hery | Note Added: 0011468 | |
| 2010-01-05 12:57 | Hery | Note Added: 0011489 | |
| 2010-01-05 12:57 | Hery | Status | assigned => resolved |
| 2010-01-05 12:57 | Hery | Fixed in Version | => 3.6.11 |
| 2010-01-05 12:57 | Hery | Resolution | open => fixed |
| 2010-01-05 13:59 | KeldorKatarn | Note Added: 0011490 | |
| 2010-01-05 13:59 | KeldorKatarn | Status | resolved => feedback |
| 2010-01-05 13:59 | KeldorKatarn | Resolution | fixed => reopened |
| 2010-01-05 15:04 | Hery | Note Added: 0011491 | |
| 2010-01-05 15:13 | KeldorKatarn | Note Added: 0011492 | |
| 2010-01-05 17:54 | Hery | Note Added: 0011494 | |
| 2010-01-05 18:00 | KeldorKatarn | Note Added: 0011495 | |
| 2010-01-05 18:04 | KeldorKatarn | Note Edited: 0011495 | |
| 2010-01-05 19:03 | Hery | Note Added: 0011496 | |
| 2010-01-05 19:08 | Hery | Note Edited: 0011496 | |
| 2010-01-05 20:14 | KeldorKatarn | Note Added: 0011497 | |
| 2010-12-09 22:41 | The_E | Note Added: 0012527 | |
| 2010-12-09 22:41 | The_E | Status | feedback => closed |
| 2010-12-09 22:41 | The_E | Resolution | reopened => suspended |
| 2010-12-09 22:41 | The_E | Fixed in Version | 3.6.11 => |
