sp614x / optifine

1.8k stars 418 forks source link

Shaders: Skip clear buffers doesn't work #424

Closed DethRaid closed 7 years ago

DethRaid commented 7 years ago

@dotmodded and I have been working on temporal raytracing by using the ability to not clear some buffers to accumulate ray samples. In doing this, we have discovered that not clearing buffers is somehow not working in the slightest.

You can see all our code at https://github.com/Continuum-Shaders/Pure_PBR/tree/broke. Specifically, check out https://github.com/Continuum-Shaders/Pure_PBR/blob/broke/shaders/composite2.fsh#L261 and on. You'll see that we read in the color from the previous frame buffer (which in our case is colortex2), mix it with the current frame's color, and write the result to colortex0, which finds its way onto the screen. You'll also see that we write a pure white color to the previous frame buffer.

The expected behavior of this is that we'll see an image that is a 50/50 mix of the current frame's color and a pure white color. What we see instead looks like motion blur. You can see a screenshot at http://i.imgur.com/5EHsDs0.png. You'll notice that you can see three or four ghosts of the lime green wool block in my hand. This result makes no sense to either me or @dotmodded, and we're really hoping you can help us out.

williammalo commented 7 years ago

reduced-test-case.zip I have the same problem

dotModded commented 7 years ago

temporal not working.zip

This should cause a tail however it does not. I have for some reason been able to get this working however.

In this example I am writing 0.5 to the buffer however it is just showing previous frame data. temporal working but not with the correct data at all.zip

zombye commented 7 years ago

Here's a minimized version of the "sort of working but wrong data" test case @dotModded provided temporal working but not really.zip

I've changed buffer formats to RGBA8 as for some reason that produces pretty much endless trails, changing them to RGBA16 or RGBA32F (and probably other formats) fixes it. I guess that might be due to reading and writing to the same buffer, but buffer formats shouldn't affect that.

zombye commented 7 years ago

I've done a bit more testing, here's what I've found out:

It actually works.

Here's what I was using: temporal actually working.zip

It appears that you need to write whatever you want on the next frame in the last composite pass before final, provided that pass is not also the first time you write that data to that buffer.

I suppose it makes some sense that you'd need to write it in the last composite pass, but you definitely shouldn't need to write it in more than one pass.

DethRaid commented 7 years ago

you need to write whatever you want on the next frame in the last composite pass before final, provided that pass is not also the first time you write that data to that buffer.

That's... very intuitive and exactly how I expect this feature to work?

So let me get this straight... I have to write to the buffer in a composite pass that is not the last composite, then I have to write to the buffer in the last composite pass, and only then will it work? Wow.

@sp614x is there any way we could see the code for this feature? I'm super confused about how it works and how you even got it to work like this

Strum355 commented 7 years ago

+1 @sp614x , we shader devs are confused about some of the features you've added in Optifine. Could you come to our Discord server and help clear some things up? We think if youre going to work on optifine and working on giving the shader community features, we very highly recommend that you join the discord server. It would be most benificial to us, as you will better understand our needs, and you will recieve faster feedback as well as more precise requests. We hope you dont ignore this, as we would very much like to work with you closer on integrating useful features for shader devs into optifine Link: https://discord.gg/RpzWN9S

sp614x commented 7 years ago

The optional clear is implemented like this:

for (int i = 2; i < usedColorBuffers; ++i)
{
  // Check clear disabled
  if(!gbuffersClear[i])
    continue;
  // Clear
  glDrawBuffers(GL_COLOR_ATTACHMENT0_EXT + i);
  glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
  glClear(GL_COLOR_BUFFER_BIT);
}
sp614x commented 7 years ago

Minimal working case, it should produce trails when moving: SkipClearWorkingMin.zip Adding additional composite programs does not affect it: SkipClearComposite3.zip Adding a final program also does not affect it: SkipClearFinal.zip

sp614x commented 7 years ago

The only limit so far seems to be that the read and write to the color buffer can not happen in the same program.

From GLSL : common mistakes: "Normally, you should not sample a texture and render to that same texture at the same time. This would give you undefined behavior. It might work on some GPUs and with some driver version but not others."

From Framebuffer Object: Feedback Loops: "It is possible to bind a texture to an FBO, bind that same texture to a shader, and then try to render with it at the same time. It is perfectly valid to bind one image from a texture to an FBO and then render with that texture, as long as you prevent yourself from sampling from that image. If you do try to read and write to the same image, you get undefined results. Meaning it may do what you want, the sampler may get old data, the sampler may get half old and half new data, or it may get garbage data. Any of these are possible outcomes. Do not do this. What you will get is undefined behavior."

sp614x commented 7 years ago

Currently the skip clear is not implemented for gcolor and gdepth (0 and 1).

sp614x commented 7 years ago

Implemented skip clear for gcolor and gdepth (0 and 1) in OptiFine 1.11.2_HD_U_B7.