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

Rendering: remove rendercaps #13512

Merged
merged 3 commits into from Feb 21, 2018

Conversation

@lrusak
Copy link
Contributor

lrusak commented Feb 10, 2018

rendercaps aren't needed. Just check the extension instead.

@FernetMenta I only did GLES. Please do GL if you want to get rid of render caps completely.

@FernetMenta

This comment has been minimized.

Copy link
Member

FernetMenta commented Feb 10, 2018

ok, will do today

@lrusak lrusak force-pushed the lrusak:rendercaps branch from 988a675 to b136639 Feb 11, 2018
@lrusak

This comment has been minimized.

Copy link
Contributor Author

lrusak commented Feb 11, 2018

jenkins build this please

@lrusak

This comment has been minimized.

Copy link
Contributor Author

lrusak commented Feb 12, 2018

@afedchin or @Paxxi mind taking care of windows?

@lrusak lrusak changed the title [GLES] Rendering: remove rendercaps Rendering: remove rendercaps Feb 12, 2018
bool CRenderSystemBase::SupportsBGRAApple() const
{
return (m_renderCaps & RENDER_CAPS_BGRA_APPLE) == RENDER_CAPS_BGRA_APPLE;
return true;

This comment has been minimized.

Copy link
@afedchin

afedchin Feb 12, 2018

Member

it's not always true for windows (see RenderingSystemDX for details). so at least we must make this method virtual.

edit: sorry, you made it virtual

{
m_textureWidth = PadPow2(m_textureWidth);
m_textureHeight = PadPow2(m_textureHeight);
}
if (m_format & XB_FMT_DXT_MASK)

This comment has been minimized.

Copy link
@afedchin

afedchin Feb 12, 2018

Member

I'm not sure that this will not break windows at least on older hw.

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Feb 12, 2018

Member

If windows requires this, we should hide it in platform code. What do you think?

This comment has been minimized.

Copy link
@afedchin

afedchin Feb 15, 2018

Member

I was wrong when saying that this breaks windows. look like this breaks support of compressed formats (XB_FMT_DXT(1|2|5)) at all, because it's a requirement for texture for these formats.
I'm not sure for OGL (I think it acts the same) but MSDN says:

Direct3D will not allow a resource to be created using BC format with the top-level size set to something other than a multiple of 4 in width and height, even though it does allow the mipchain below it to not meet that requirement.

if we drop this we also must drop switch at https://github.com/xbmc/xbmc/pull/13512/files#diff-a438d95a90f1e375829fdccadc6f1e1cL122
I'm also not sure that this will work if it will try to create a texture with GL_COMPRESSED_RGBA_S3TC_DXT1_EXT format via glTexImage2D instead of glCompressedTexImage2D

This comment has been minimized.

Copy link
@afedchin

afedchin Feb 15, 2018

Member

I'm not against removing render caps, I want to do this right. I think removing XB_FMT_DXT_MASK is wrong. because we do support of DDS images which can be in compressed format.

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Feb 16, 2018

Member

I missed this as I searched the code for this. I'll put it back

@lrusak

This comment has been minimized.

Copy link
Contributor Author

lrusak commented Feb 19, 2018

jenkins build this please

@afedchin

This comment has been minimized.

Copy link
Member

afedchin commented Feb 19, 2018

rebase required to build

@lrusak lrusak force-pushed the lrusak:rendercaps branch from cf1719f to 5eca96f Feb 20, 2018
@lrusak lrusak force-pushed the lrusak:rendercaps branch from 5eca96f to 185ddcf Feb 20, 2018
@lrusak

This comment has been minimized.

Copy link
Contributor Author

lrusak commented Feb 20, 2018

jenkins build this please

@lrusak

This comment has been minimized.

Copy link
Contributor Author

lrusak commented Feb 20, 2018

build failure not related. good to go?

@lrusak lrusak merged commit 8976083 into xbmc:master Feb 21, 2018
1 check failed
1 check failed
default Sorry, building this PR failed. Please check the logs for the errors.
Details
@Rechi Rechi added this to the L 18.0-alpha1 milestone Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.