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

Add depthTest along with stencilTest since depth Fail/ZPass test used . #3598

Merged
merged 1 commit into from
Sep 3, 2013
Merged

Conversation

dbz400
Copy link
Contributor

@dbz400 dbz400 commented Sep 3, 2013

Fix missing character in Star Ocean and FF 4 complete collection in buffered-rendering mode.

#1392 , #2018

Already tested with 20 other titles and seems to be fine.

@dbz400
Copy link
Contributor Author

dbz400 commented Sep 3, 2013

This somehow fixes the graphical issue in GOW as well .#3046

@brujo5
Copy link

brujo5 commented Sep 3, 2013

fix saint seiya omega?

@dbz400
Copy link
Contributor Author

dbz400 commented Sep 3, 2013

No, Saint Seiya Omega one is another issue.

@solarmystic
Copy link
Contributor

@raven02

You can't claim to have fixed #2018 since it's an AMD/ATI specific issue for FF 4 and it cannot be reproduced on your NVIDIA card. If I remember correctly, you're using a GTX 690 for your test PC.

I'll have to test this myself, which I will do abit later when I return home. Will provide the usual table of results as well.

Other AMD/ATI users results will also be very helpful to confirm that the issue is resolved.

-Posted from my phone-

@dbz400
Copy link
Contributor Author

dbz400 commented Sep 3, 2013

Yep , but that one is the best related i can find for a open issue still for FF 4 CC. Feel free to test it to see if any games breaks by it.

@solarmystic
Copy link
Contributor

@raven02

Got back home, went straight to testing. I'm just speechless right now, you've attained the breakthrough we long suffering ATI/AMD card owners have been waiting for.

Here're are the results, tabulated for your perusal.

capture

Major Observations:-

a. Congratulations on the FF 4 fix for #2018. AMD/ATI cards now work with Text properly displayed with your proposed change in the commit! I'm really impressed that such a small change could fix this very longstanding issue!

screen00041

b. The world map is flickering for Tactics Ogre in buffered rendering mode now when you first enter it from loading a savegame. Fortunately for us, this is an unrelated issue that is also present in the latest master that I checked (v0.9.1-487-g728e1a5). Will do some bisecting to figure out the first responsible build later and perhaps open up a seperate issue report for it, but I will reiterate that it has nothing to do with your commit since it's already present in the latest master.

c. Untold Legends background is somehow darker with this commit, as compared to the latest master. I checked it again to make sure I wasn't seeing things and took screenshots to compare. Nothing is graphically broken though. The characters still display properly and the minimap is working as usual, just thought I should point that out. Perhaps the actual scene is meant to be darker on the PSP.

Master (brighter):-
screen00039

Test build with your commit (slightly darker):-
screen00040

Other than the observations above, I've nothing else to add other than yet another big thank you for finally resolving the FF 4 text issue that has been plaguing AMD/ATI cards for that game for a very, very long time now.

@papel
Copy link

papel commented Sep 3, 2013

It is not darker on PSP, unless the game has night/day cycle.
http://www.youtube.com/watch?v=zPCdUFG3bTg#t=144

@solarmystic
Copy link
Contributor

@papel

Thanks for the video. Hmm.. I guess it's not meant to be that dark then. Other than that very minor observation for Untold Legends, every other game I've tested checks out nicely.

@solarmystic
Copy link
Contributor

@raven02 @hrydgard @unknownbrackets

The actual cause of the world map flickering in Tactics Orge has been discovered, and as I've mentioned earlier, it's completely unrelated to this commit by @raven02 #2758 (comment)

@thedax
Copy link
Collaborator

thedax commented Sep 3, 2013

Fixes FF4's text on NVidia as well. Now it can display text with BR on.

@unknownbrackets
Copy link
Collaborator

Well, the question I guess is if it's right, or if it's just disabling stencil tests that we're not supporting well... need tests.

Can't test atm, but does this affect GEB/Wipeout/etc. that are normally too bright? I think that was supposed to be a stencil issue as well?

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Sep 3, 2013

I think the old difference between buffered/unbuffered is because on Windows, we happen to create a context without a stencil buffer on nvidia but on ATI, the driver doesn't obey and we get one anyway. And then things go wrong, possibly because of this issue, if this is in fact correct.

I'm willing to try this for a bit and I will also try to change the GL init soon so that we always get a stencil buffer.

hrydgard added a commit that referenced this pull request Sep 3, 2013
Add depthTest along with stencilTest since depth Fail/ZPass test used .
@hrydgard hrydgard merged commit 09c07a8 into hrydgard:master Sep 3, 2013
@papel
Copy link

papel commented Sep 3, 2013

Wipeout Pulse continues with overbright issue.

@solarmystic
Copy link
Contributor

@unknownbrackets @raven02 @hrydgard

Confirming @papel's observations. It's the same with GEB, which still has the overbright issue also with Buffered Rendering.

Sigh, on the other hand, it seems that I can replicate the "darker" background effect with Untold Legends in another game (Brave Traveller: New Story (ULUS 10279)) as well, which unfortunately wasn't a part of my usual test suite which is why I didn't catch it the first time around.

Before the commit (shadows are present, the scene is bright and cheery)
screen00045
screen00047

After the commit (missing shadows, the scene is unnaturally dark)
screen00046
screen00048

Different enemy encounters in the screenshots, but I think you get the picture. The shadows are missing and the whole picture is dark.

Discounting the rectangular artifacts on the screen, which were present in the master before the commit, I don't think the game is supposed to look that dark either.

@papel
Copy link

papel commented Sep 3, 2013

I think this new problem should be moved to a new issue thread, because it is more complex.

@solarmystic
Copy link
Contributor

@papel

It is a problem caused by this commit though, which is why I've put it here. I'll make a new one anyway if you think it's necessary.

@unknownbrackets
Copy link
Collaborator

Thanks for the clear comparison screenshots. Seeing it like that makes it pretty clear how shadows and stencil tests are interacting. I think this is why Adventures To Go is overdark, it has always looked like Brave Story now does after this pull.

It could be that this change is wrong, or it could be that it is right but there's something else wrong.

-[Unknown]

@papel
Copy link

papel commented Sep 4, 2013

The status of this thread is "closed" because it is was merged. A new thread would have the status as "open" and it would help to remind that this bug still exists. If this commit is correct, it may be difficult to find where the error is.

@dbz400
Copy link
Contributor Author

dbz400 commented Sep 4, 2013

I did observe the "into the darkness" issue in some games but not all of the games .Probably this change not in favour to some games and as @unknownbrackets mentioned may be this change just simply disable stencil testing in some cases.I will take further look on it to see if can get a better result for it.

@hrydgard
Copy link
Owner

hrydgard commented Sep 4, 2013

I think it's pretty clear that this isn't the right fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants