Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3 textures blending fix #710

Merged
merged 3 commits into from Nov 4, 2014

Conversation

lukaspj
Copy link
Contributor

@lukaspj lukaspj commented Jun 30, 2014

While working with the terrain blending, I found some artifacts when having more than 2 textures blended together.
Steve_Yorkshire in IRC confirmed that he had also seen the issue, so I dug into it and found that the textures aren't blended properly. This PR fixes specifically the blending between the 2nd and the third texture.

Image of the issue here:
Blend artifacts

With this PR it looks like this:
Blend fixed

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Jul 1, 2014

Looks sweet. Haven't reviewed the code yet. Not sure how my commit snuck its way into your branch. Maybe basing off an old head? The build status icon was reverted a while ago.

EDIT: Hmm. On a cursory glance there are a lot of whitespace changes...

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jul 1, 2014

Is it the whitespaces again? Dammit! My IDE wants to be smart and does that stuff on its own :(

I'll be removing stupid commits and whitespaces again.. sigh

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Jul 1, 2014

Cheers! Yeah I wanted to ignore all that because I prefer your whitespace settings to the existing format, but it made it very difficult to actually check the PR since most lines were touched :P.

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Jul 1, 2014

Summary of IRC discussion: This fix is specific to 3 textures, if we ever had 4 textures blending at once we'd still have trouble. I personally would be interested in seeing a general solution. @lukaspj agreed to look into it.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jul 1, 2014

@eightyeight To be fair I actually never answered you... I kind of ignored you.. To no avail it seems.. ;)

Fixed it now though! Generalized and might work for more textures.

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Jul 1, 2014

Oh, I thought you said you would do it! Maybe I'm psychic. EDIT: I quote:

I'll try and do a general approach :P See if it works

Anyway, if it's proven to work for 3 textures, then it's at least as good as the previous non-generic fix!

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jul 1, 2014

Nah you just read my thoughts silly!
Which is good because I have a tendency of not answering when I bury my head in work :P

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jul 1, 2014

Making a small change to this stand by..
Edit: There! Just a change that will simplify future pull requests in this area, also isolates the changes more.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jul 1, 2014

Doing another fix, an unforeseen case where detailBlend < 0 gives some weird issues, this change should give the best blending possible :P

@Azaezel
Copy link
Contributor

@Azaezel Azaezel commented Aug 16, 2014

should also note, needed to default that detailBlend%d to something in order to prevent crashes this end when porting it on over for cases where detailCount = 0.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Aug 22, 2014

This PR has a major issue, as I misunderstood how the terrain renders the materials.. Basically I think I'll need change how the blending values are calculated because there are issues when several different shaders are run over the same pixel because it introduces loss of information..

This is also relevant for #712 and #714

@lukaspj lukaspj force-pushed the 3-Textures-Blending-Fix branch 2 times, most recently from ff8623e to 7f475fb Compare Aug 24, 2014
@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Aug 25, 2014

Okay, I think I pinpointed the issue in the calcBlend method of the terrain texture.
As a result I was able to make minimal changes to terrain.hlsl to achieve the same fix.

Before

before

After

after

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Aug 25, 2014

Looks fantastic!

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Aug 25, 2014

Fixed the color loss in the above 2 images:
Blending4

Now it blends properly, and normal maps are working too and everything. I'll be finishing this PR up shortly.

@lukaspj lukaspj force-pushed the 3-Textures-Blending-Fix branch from d3018f0 to 4659d67 Compare Aug 25, 2014
Azaezel added a commit to Azaezel/Torque3D that referenced this issue Aug 25, 2014
@crabmusket crabmusket added this to the 3.6 milestone Aug 25, 2014
@lukaspj lukaspj force-pushed the 3-Textures-Blending-Fix branch from 4659d67 to 53f2419 Compare Aug 26, 2014
@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Aug 27, 2014

Worth noting, this PR also fixes the ´visual result of the terrain textures, because you get the expected results instead of the previous.
Previously, the first texture would blend in pretty much everywhere, meaning it had an impact on terrain color, even when it shouldn't..

Tl;DR textures generally look better:

Before

Before

After

After

@Azaezel Azaezel mentioned this pull request Aug 29, 2014
@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Sep 5, 2014

Okay, so I fetched your repo, merged this branch and the editor branch into development head, went into Empty Terrain, painted some 'grass' on it, restarted the engine and this is what it looks like:

terrain_blending

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Sep 5, 2014

@eightyeight that issue dissappear is you paint something on the terrain right?
Hmm I will go in the thinking box, with my thinking hat..

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Sep 5, 2014

Yep, forgot to mention that :P.

crabmusket added a commit that referenced this issue Nov 4, 2014
@crabmusket crabmusket merged commit 9e44460 into GarageGames:development Nov 4, 2014
1 check passed
@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 7, 2014

I think that you should reconsider this modification. I've added it to my build which also includes the D3D9 refactor and it didn't give a very nice result.
Here is an image without this change:
screenshot_003-00000

And here is more or less the same situation with these changes added:
screenshot_002-00002

Notice that these changes introduced a very distinct separation where 2 textures blend (for convenience I've marked it with arrows).

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 7, 2014

Hmm that shouldn't happen, though I recognize the issue.. I'll look into it.

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Nov 7, 2014

@RichardsGameStudio great to know you're merging changes from development into your game - it's really helpful to get tests/reports like this! I'll see if I can reproduce what you're seeing.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 7, 2014

You're welcome.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 7, 2014

@RichardsGameStudio could you upload the basetexture generated? It should be next to the terrain file.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 8, 2014

I think I found the issue. Seems like it would only apply to textures rendered in the same batch.
Basically this:

OUT.col = lerp( OUT.col, baseColor + detailColor, detailBlend1 );

Should be this

OUT.col += detailColor * detailBlend1;

In the generated shaders I believe. Seems like it fixes the issue on my end.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 8, 2014

@RichardsGameStudio could you test the change at #920 ? Then you'd make me a happy developer :P

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 8, 2014

@lukas,

I have tried the changes you suggested, but I am sorry to say that the issue still exists.

In any case the basetex file is included with this email

Thanks
Richard.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 8, 2014

@lukaspj I have replied directly to your email which includes the basetex file. I've tried the suggested changes at #920 but it still gave the issue.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 8, 2014

Have you made sure to delete the basetex file before testing it? To ensure it gets generated again.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 8, 2014

I didn't, but now I have, still having the same issue.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 10, 2014

Okay, my next guess is trying to clean the shaders and see if that works. I replicated the issue on my end and fixed it.. Can't figure out how to replicate it after that fix.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Nov 11, 2014

After cleaning the shaders still no improvement. Are you using the D3D9 refactor? Otherwise it' might be a hardware issue?

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 11, 2014

No I pulled it straight out of the development branch. Hmm.. What about the generated shaders? Just every terrain)related shader, could you send that to me somehow?

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Nov 11, 2014

@RichardsGameStudio can you reproduce this in the development branch, or would it be too much effort to test it?

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Nov 19, 2014

I believe that to reproduce this, you have to use MacroTextures.. But it's all pretty weird and fuzzy.

@RichardsGameStudio do you use macrotextures? And what happens if you turn macro off?

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Jan 12, 2015

@RichardsGameStudio It's been a while, but in case you are still available, could you have a look at #920? I just added a new commit with some new changes in it. I've not been able to reproduce the seams after those changes.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Feb 6, 2015

@lukaspj Sorry for the late reaction, but it got into the pile of things I had to do and it was growing faster than I realized. I will try to find some time this weekend to test it.

@crabmusket
Copy link
Contributor

@crabmusket crabmusket commented Feb 6, 2015

That'd be great :). @lukaspj would you be able to put all the terrain blending changes into a single branch for easier testing? Separate PRs was good to review each of them individually, but I've started to forget which order they happen in and which ones need to be merged :P.

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Feb 8, 2015

@lukaspj I've tried the suggested solution, deleted all the procedural shaders, deleted the basetex file, but still the same fuzzy borders. I am using terrainmaterials with all the maps set (diffuse, macro, detail and normal).

@crabmusket crabmusket added this to the 3.8 milestone Feb 8, 2015
@crabmusket crabmusket removed this from the 3.7 milestone Feb 8, 2015
@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Feb 8, 2015

@RichardsGameStudio I'd love to get a minimal working example, like if you could reproduce the effect in the empty terrain level and send the whole project to me!

@RichardsGameStudio
Copy link
Contributor

@RichardsGameStudio RichardsGameStudio commented Feb 16, 2015

@lukaspj I'll see what I can do. The problem is that there are parts of the engine that I have modified which I can not make public, but as said I'll see what I can do for you.

@lukaspj
Copy link
Contributor Author

@lukaspj lukaspj commented Feb 16, 2015

@RichardsGameStudio I don't need the actual source code.. Just like, a mission with a flat terrain and textures which can be used to reproduce the issue. I'd use the stock repo to test the mission anyway, as I only need to test the terrain rendering :)

Edit: something like this actually: this

@lukaspj lukaspj deleted the 3-Textures-Blending-Fix branch Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants