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

GSdx: Automatic HW Mipmapping #2099

Merged
merged 1 commit into from Nov 20, 2017

Conversation

@lightningterror
Member

lightningterror commented Oct 16, 2017

Adds automatic HW mipmapping support.
It relies on crc ids so if a game does
not have their crc id but needs mipmapping
it will not work until the id is added.

Add GUI menu and tooltip for Automatic mipmap
"Automatic (Default)"
This option will be default option from now on.

Rename "Very Slow" text option to "Slow" for full mipmap
as it caused the text not to fit properly in
the menu.

Credits also go to @RedPanda4552 and @ssakash for helping with the code.

Note: Not all games are added. So it needs a bit of work.

@MrCK1

I'm still not in favor of implementing this personally, but the code seems to work OK for the most part.

Show outdated Hide outdated plugins/GSdx/GSdx.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererOGL.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSSetting.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSDeviceOGL.h Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSCrc.h Outdated
Show outdated Hide outdated plugins/GSdx/GSCrc.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSDeviceOGL.cpp Outdated
@atomic83GitHub

This comment has been minimized.

Show comment
Hide comment
@atomic83GitHub

atomic83GitHub Oct 16, 2017

Contributor

Don't forget Harry poter games, Gran tourismo 4 and Half life :).

Contributor

atomic83GitHub commented Oct 16, 2017

Don't forget Harry poter games, Gran tourismo 4 and Half life :).

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Oct 16, 2017

Member

Gran Turismo 4 does not need mipmapping. I'm not sure about Harry Potter games.

Member

lightningterror commented Oct 16, 2017

Gran Turismo 4 does not need mipmapping. I'm not sure about Harry Potter games.

@atomic83GitHub

This comment has been minimized.

Show comment
Hide comment
@atomic83GitHub

atomic83GitHub Oct 16, 2017

Contributor

Chamber of secrets and the Half blood prince massively use it.

Contributor

atomic83GitHub commented Oct 16, 2017

Chamber of secrets and the Half blood prince massively use it.

@MrCK1

This comment has been minimized.

Show comment
Hide comment
@MrCK1

MrCK1 Oct 16, 2017

Member

@lightningterror You're wrong about GT4. While there is no corruption if mipmapping is off, it's required to reduce texture flickering (which is what mipmapping is intended for anyways)

Member

MrCK1 commented Oct 16, 2017

@lightningterror You're wrong about GT4. While there is no corruption if mipmapping is off, it's required to reduce texture flickering (which is what mipmapping is intended for anyways)

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Oct 16, 2017

Member

Which version pal or ntsc?
For me on pal it actually causes texture flickering.

Member

lightningterror commented Oct 16, 2017

Which version pal or ntsc?
For me on pal it actually causes texture flickering.

@atomic83GitHub

This comment has been minimized.

Show comment
Hide comment
@atomic83GitHub

atomic83GitHub Oct 16, 2017

Contributor

I am with the Pal fr version, seeting these 2 Harry poter games with Mipmapping basic solve some incorect distant texture but set mipmapping to full cause texture flickering.

Contributor

atomic83GitHub commented Oct 16, 2017

I am with the Pal fr version, seeting these 2 Harry poter games with Mipmapping basic solve some incorect distant texture but set mipmapping to full cause texture flickering.

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Oct 16, 2017

Member

This is what mipmapping causes on the ground textures on GT4. OpenGL and Direct3D so I won't add it if it fixes but breaks something else.

https://drive.google.com/open?id=0B8cQAq-Ko9-uMFV5T1hxVW9QRUU

Let's just stick with games where mipmapping is essential like Ace combat , R&C and other games with similar issues.

Member

lightningterror commented Oct 16, 2017

This is what mipmapping causes on the ground textures on GT4. OpenGL and Direct3D so I won't add it if it fixes but breaks something else.

https://drive.google.com/open?id=0B8cQAq-Ko9-uMFV5T1hxVW9QRUU

Let's just stick with games where mipmapping is essential like Ace combat , R&C and other games with similar issues.

@MrCK1

This comment has been minimized.

Show comment
Hide comment
@MrCK1

MrCK1 Oct 16, 2017

Member

That shouldn't be happening, IIRC Full breaks this game. Try half instead.

Member

MrCK1 commented Oct 16, 2017

That shouldn't be happening, IIRC Full breaks this game. Try half instead.

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Oct 16, 2017

Member

The video is from Half/Basic mipmap. Full does the same.

edit: The brighter texture is properly rendered on Software mode with mipmapping enabled. HW mode without mipmap the brighter texture is not there. Mipmap tries to emulate the brighter texture but not properly which causes that flickering.
Note it also causes massive slowdowns on corners.

tl-dr: mipmapping isn't accurate on GT4 and causes some issues.

Member

lightningterror commented Oct 16, 2017

The video is from Half/Basic mipmap. Full does the same.

edit: The brighter texture is properly rendered on Software mode with mipmapping enabled. HW mode without mipmap the brighter texture is not there. Mipmap tries to emulate the brighter texture but not properly which causes that flickering.
Note it also causes massive slowdowns on corners.

tl-dr: mipmapping isn't accurate on GT4 and causes some issues.

@mirh

This comment has been minimized.

Show comment
Hide comment
@mirh

mirh Oct 17, 2017

This is what mipmapping causes on the ground textures on GT4. OpenGL and Direct3D so I won't add it if it fixes but breaks something else.

The determinant should be whether you broke more than what you fix, tbh.

Anyway, this should be expanded to include just about everything else too imo #1894
EDIT: hashtags #globalAUTOsetting #happyn00bs #dolphin

mirh commented Oct 17, 2017

This is what mipmapping causes on the ground textures on GT4. OpenGL and Direct3D so I won't add it if it fixes but breaks something else.

The determinant should be whether you broke more than what you fix, tbh.

Anyway, this should be expanded to include just about everything else too imo #1894
EDIT: hashtags #globalAUTOsetting #happyn00bs #dolphin

@MrCK1

This comment has been minimized.

Show comment
Hide comment
@MrCK1

MrCK1 Oct 17, 2017

Member

I'm looking at the code again and I keep thinking that this might be breaking more games. If a CRC is already in the list for CRC hacks, mipmapping may be wrongly applied automatically and break more.

Member

MrCK1 commented Oct 17, 2017

I'm looking at the code again and I keep thinking that this might be breaking more games. If a CRC is already in the list for CRC hacks, mipmapping may be wrongly applied automatically and break more.

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Oct 17, 2017

Member

All the code does is read the crc id of the game and applies mipmapping if it's on the list. Nothing else.
If a game already has crc hacks then they will function the same.

Member

lightningterror commented Oct 17, 2017

All the code does is read the crc id of the game and applies mipmapping if it's on the list. Nothing else.
If a game already has crc hacks then they will function the same.

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Oct 17, 2017

Contributor

Looking at the code, it should be better to do something like that (in GSRendererHW.cpp)

void GSRendererHW::SetGameCRC(uint32 crc, int options)
{
	GSRenderer::SetGameCRC(crc, options);

	m_hacks.SetGameCRC(m_game);

      if (m_mipmap == AUTO_MODE)  {
                switch (m_game.title.title) 
                       case game1:
                       case game2:
                                  m_mipmap = ON;
                        break;
                        default:
                                 m_mipmap = OFF:
     }
}

It avoids to create new variable and to put more code in the rendering loop. (Note, you can extend the above with FULL mode)

Edit: bonus point for creating a nice enum :)

Contributor

gregory38 commented Oct 17, 2017

Looking at the code, it should be better to do something like that (in GSRendererHW.cpp)

void GSRendererHW::SetGameCRC(uint32 crc, int options)
{
	GSRenderer::SetGameCRC(crc, options);

	m_hacks.SetGameCRC(m_game);

      if (m_mipmap == AUTO_MODE)  {
                switch (m_game.title.title) 
                       case game1:
                       case game2:
                                  m_mipmap = ON;
                        break;
                        default:
                                 m_mipmap = OFF:
     }
}

It avoids to create new variable and to put more code in the rendering loop. (Note, you can extend the above with FULL mode)

Edit: bonus point for creating a nice enum :)

Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSSetting.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSdx.cpp Outdated

@PCSX2 PCSX2 deleted a comment from MrCK1 Nov 9, 2017

@PCSX2 PCSX2 deleted a comment from lightningterror Nov 9, 2017

Show outdated Hide outdated plugins/GSdx/GSdx.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
@turtleli

I think there is more code that you could use the new enum in (search m_mipmap and convert if appropriate).

Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSState.h Outdated
Show outdated Hide outdated plugins/GSdx/GSSetting.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
Show outdated Hide outdated plugins/GSdx/GSRendererHW.cpp Outdated
@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Nov 10, 2017

Member

I just found this out. Suggestions ?
Should I move the game to Basic Mipmapping ?
#1853 (comment)

edit: I removed the game just in case.

Member

lightningterror commented Nov 10, 2017

I just found this out. Suggestions ?
Should I move the game to Basic Mipmapping ?
#1853 (comment)

edit: I removed the game just in case.

GSdx: Automatic HW Mipmapping option
Adds automatic HW mipmapping support.
It relies on CRC ids so if a game does
not have their CRC id but needs mipmapping
it will not work until the id is added.

Add GUI menu and tooltip for Automatic mipmap
"Automatic (Default)"
This option will be default option from now on.

Rename "Very Slow" text option to "Slow" for full mipmap
as it caused the text not to fit properly in the menu.

Credits also go to @RedPanda4552 and @ssakash for helping
with the code.

@gregory38 gregory38 added the Reviewed label Nov 13, 2017

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Nov 13, 2017

Contributor

Code is fine for me. Is everybody happy ? Otherwise code could be merged

Contributor

gregory38 commented Nov 13, 2017

Code is fine for me. Is everybody happy ? Otherwise code could be merged

@RedPanda4552

This comment has been minimized.

Show comment
Hide comment
@RedPanda4552

RedPanda4552 Nov 13, 2017

Contributor

I did notice that there's no longer a condition for Full mipmap games in the switch. Was a decision made to scrap that, that I haven't seen in here?

Contributor

RedPanda4552 commented Nov 13, 2017

I did notice that there's no longer a condition for Full mipmap games in the switch. Was a decision made to scrap that, that I haven't seen in here?

@lightningterror

This comment has been minimized.

Show comment
Hide comment
@lightningterror

lightningterror Nov 13, 2017

Member

There are no games on the Full list atm so there is no reason to have it. If a game pops up it will be added ofc.
Ape Escape was removed since it seems there are issues with DX and trilinear filtering not supported with dx. (Ape Escape requires Mipmap Full + Trilinear(ogl only).

Member

lightningterror commented Nov 13, 2017

There are no games on the Full list atm so there is no reason to have it. If a game pops up it will be added ofc.
Ape Escape was removed since it seems there are issues with DX and trilinear filtering not supported with dx. (Ape Escape requires Mipmap Full + Trilinear(ogl only).

@gregory38 gregory38 merged commit 6d49bf6 into PCSX2:master Nov 20, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lightningterror lightningterror deleted the lightningterror:automaticmipmap branch Nov 20, 2017

@gregory38 gregory38 referenced this pull request Jan 21, 2018

Open

Meta: GSdx HW mipmap support #1561

16 of 28 tasks complete
@Temporus

This comment has been minimized.

Show comment
Hide comment
@Temporus

Temporus Jul 10, 2018

The Incredible Hulk: Ultimate Destruction need Basic Mipmapping for texture issues on buildings

Temporus commented Jul 10, 2018

The Incredible Hulk: Ultimate Destruction need Basic Mipmapping for texture issues on buildings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment