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

[pre-ll] Add BC1 (DXT1), BC3 (DXT5) S3TC texture formats #2733

Merged
merged 4 commits into from Apr 22, 2019

Conversation

Projects
None yet
3 participants
@alexheretic
Copy link
Contributor

commented Apr 9, 2019

Adds compressed BC1 & BC3 image formats for gfx_device_gl/opengl. Image data is expected to be already compressed.

New formats:

  • SurfaceType::BC1_R8_G8_B8 (DXT1)
  • SurfaceType::BC3_R8_G8_B8_A8 (DXT5)

Each supports rgb & srgb.

Resolves #2688

Motive

I've been investigating texture compression to reduce VRAM usage in my game Robo Instructus. For this end I wanted more efficient 8-bit channel rgb & rgba storage with good opengl driver & hardware support.

S3TC is old and the best supported compression available (see https://opengl.gpuinfo.org/listcompressedformats.php). Patent issues are now history (see https://en.wikipedia.org/wiki/S3_Texture_Compression). For testing I used GPUOpen-Tools/Compressonator for encoding, which is quite fast and seems to have decent quality.

  • BC1_R8_G8_B8 offers 8:1 compression over R8_G8_B8_A8
  • BC3_R8_G8_B8_A8 offers 4:1 compression over R8_G8_B8_A8

I didn't include BC2/DXT3, or BC7 as I don't plan to use it. However, support for this format can be trivially added on top of this change.

No ETC2?

I did investigate ETC2 compression too which is newer yet features wide support. However, I found actual hardware support for ETC2 on the desktop is much more limited that it may appear. It was a bit of a downer to see no VRAM usage reduction on my polaris RX480 when using ETC2 compressed textures. ETC2 formats also didn't seem to work without the GL_ARB_texture_storage/immutable storage code paths, whereas I've tested the BC1 & BC3 formats working either way. That can probably be addressed, but I didn't.

For the above reasons I decided against using ETC2 in the game & adding it here. Note though; similarly to BC2, this change will make it simpler to support ETC2 formats in future if desired.

Issues

OpenGL Extensions

This code requires S3TC extensions added to gfx_gl. The changes there are ready to go if this PR is wanted. The gfx_gl dependency line needs to be changed:
gfx-rs/gfx_gl#38
Extensions now available in gfx_gl 0.6.

API support

Robo Instructus is an OpenGL 3.3 game so I'm not personally interested in other API support for these formats at the moment. Also considering pre-ll code is in support mode, and unsupported formats already seem common I thought it wasn't a big deal not supporting other APIs.

Naming

I went with Vulkan naming style ie prefix with BC1. Maybe you guys have a better idea of what you'd like to name the formats. I'm not hugely fussed here.

Format documentation

It would be nice to have documentation for the new formats. Mentioning there various other pseudonyms & API support will probably help people wondering what BC1 does & means. However, the impl_formats! macro prevents easy documentation. I've added this. Other formats can now be fairly easily documented too.

@alexheretic alexheretic marked this pull request as ready for review Apr 9, 2019

@alexheretic alexheretic force-pushed the alexheretic:pre-ll branch from 5976343 to 2c122a7 Apr 12, 2019

@alexheretic alexheretic referenced this pull request Apr 12, 2019

Merged

Add S3TC extensions #38

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

Added some docs

@msiglreith
Copy link
Member

left a comment

Thanks,
gfx_gl needs to be changed as you mentioned

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

gfx_gl needs to be changed as you mentioned

Yep with gfx-rs/gfx_gl#38 merged & released I can update to refer to that gfx_gl version.

bors bot added a commit to gfx-rs/gfx_gl that referenced this pull request Apr 22, 2019

Merge #38
38: Add S3TC extensions r=alexheretic a=alexheretic

This update brings `gl_generator` up to date and adds extensions:
* `GL_EXT_texture_compression_s3tc`
* `GL_EXT_texture_sRGB`

Required/justified by gfx-rs/gfx#2733

Co-authored-by: Alex Butler <alexheretic@gmail.com>

alexheretic added some commits Apr 9, 2019

Add BC1 (DXT1), BC3 (DXT5) S3TC texture formats
Compressed image formats supported in gfx_device_gl
Disable travis Windows caching
Caching on Windows is slow & can cause timeout failures

@alexheretic alexheretic force-pushed the alexheretic:pre-ll branch from a475337 to 9878d14 Apr 22, 2019

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Ok the extensions are in gfx_gl and documentation for the new formats is there. I don't see only supporting OpenGL as an issue, as other backends could be added later anyway.

I've been using these formats for all the textures in Robo Instructus for the last two beta updates and everything seems fine with my testers.

Let me know if you have better ideas for the format names, otherwise this is ready to go I think.

@alexheretic alexheretic referenced this pull request Apr 22, 2019

Merged

[pre-ll] Update glutin -> 0.21 #2742

1 of 1 task complete
@kvark

kvark approved these changes Apr 22, 2019

Copy link
Member

left a comment

This looks great, amazing work!
Left one suggestion. Feel free to merge otherwise.
bors delegate+

fn is_compressed(&self) -> bool;
}

impl CompressionAware for core::format::SurfaceType {

This comment has been minimized.

Copy link
@kvark

kvark Apr 22, 2019

Member

this logic should probably be in gfx_core crate since it's not GL specific

This comment has been minimized.

Copy link
@alexheretic

alexheretic Apr 22, 2019

Author Contributor

Yeah that probably will make sense when the formats are supported in more backends. Right now this is really just an implementation detail of the gl backend, not part of the public API. I'm not sure if it even will be useful in other backends, so it makes sense to leave it private for now (it's not exactly complicated either).

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

✌️ alexheretic can now approve this pull request

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

bors r+

bors bot added a commit that referenced this pull request Apr 22, 2019

Merge #2733
2733: [pre-ll] Add BC1 (DXT1), BC3 (DXT5) S3TC texture formats r=alexheretic a=alexheretic

Adds compressed BC1 & BC3 image formats for gfx_device_gl/opengl. Image data is expected to be already compressed.

New formats:
* `SurfaceType::BC1_R8_G8_B8` (DXT1)
* `SurfaceType::BC3_R8_G8_B8_A8` (DXT5)

Each supports rgb & srgb.

Resolves #2688

## Motive
I've been investigating texture compression to reduce VRAM usage in my game Robo Instructus. For this end I wanted more efficient 8-bit channel rgb & rgba storage with good opengl driver & hardware support.

S3TC is old and the best supported compression available (see https://opengl.gpuinfo.org/listcompressedformats.php). Patent issues are now history (see https://en.wikipedia.org/wiki/S3_Texture_Compression). For testing I used [GPUOpen-Tools/Compressonator](https://github.com/GPUOpen-Tools/Compressonator) for encoding, which is quite fast and seems to have decent quality.

* `BC1_R8_G8_B8` offers 8:1 compression over `R8_G8_B8_A8`
* `BC3_R8_G8_B8_A8` offers 4:1 compression over `R8_G8_B8_A8`

I didn't include BC2/DXT3, or BC7 as I don't plan to use it. However, support for this format can be trivially added on top of this change.

## No ETC2?
I did investigate ETC2 compression too which is newer yet features wide support. However, I found actual _hardware_ support for ETC2 on the desktop is much more limited that it may appear. It was a bit of a downer to see no VRAM usage reduction on my polaris RX480 when using ETC2 compressed textures. ETC2 formats also didn't seem to work without the `GL_ARB_texture_storage`/immutable storage code paths, whereas I've tested the BC1 & BC3 formats working either way. That can probably be addressed, but I didn't.

For the above reasons I decided against using ETC2 in the game & adding it here. Note though; similarly to BC2, this change will make it simpler to support ETC2 formats in future if desired.

## Issues
### OpenGL Extensions
~This code requires S3TC extensions added to `gfx_gl`. The changes there are ready to go if this PR is wanted. The gfx_gl dependency line needs to be changed:~
~gfx-rs/gfx_gl#38
Extensions now available in gfx_gl `0.6`.

### API support
Robo Instructus is an OpenGL 3.3 game so I'm not personally interested in other API support for these formats at the moment. Also considering pre-ll code is in support mode, and unsupported formats already seem common I thought it wasn't a big deal not supporting other APIs.

### Naming
I went with Vulkan naming style ie prefix with BC1. Maybe you guys have a better idea of what you'd like to name the formats. I'm not hugely fussed here.

### Format documentation
~It would be nice to have documentation for the new formats. Mentioning there various other pseudonyms & API support will probably help people wondering what BC1 does & means. However, the `impl_formats!` macro prevents easy documentation.~ I've added this. Other formats can now be fairly easily documented too.

Co-authored-by: Alex Butler <alexheretic@gmail.com>
@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

@kvark can this be released in v0.18? I guess gfx-core will need a breaking bump, but I'm not sure if gfx "soaks" that bump or has to step with it.

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

@bors bors bot merged commit 9878d14 into gfx-rs:pre-ll Apr 22, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kvark

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

@alexheretic unfortunately, no. The format stuff is re-exported from gfx crate, and there is a few more surface types now in the enum...

Saying that, it would only be breaking for code that has a comprehensive match statement across the surface types, which I don't know any (for pre-ll), so in practice this would be a breaking change, especially considering that 0.18 was released relatively recent and hasn't gotten that many users.

Therefore, my weak preference would be on releasing this as a patch to 0.18 rather than the new version. @msiglreith @grovesNL @alexheretic opinions?

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Yes I'd have thought the only users matching on all surface types would be the gfx backends themselves. That along with the fairly recent v0.18 version I'd also favour sticking with 0.18 & yanking on complaint.

But either way will work in the end.

@alexheretic

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

I've raised #2755 as a minimally breaking release proposal.

bors bot added a commit that referenced this pull request Apr 23, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 23, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 23, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 23, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 24, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 25, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 25, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>

bors bot added a commit that referenced this pull request Apr 25, 2019

Merge #2755
2755: Texture compression pre-ll -> v0.18 r=kvark a=alexheretic

This change merges in pre-ll changes into v0.18, as the simplest method of getting BCn texture format & glutin 0.21 support there released.

Includes
* #2684
* #2722 
* #2734
* #2745 
* #2742
* #2736
* #2733

The addition of the BC1 & BC3 texture `SurfaceType`s is technically a breaking change for gfx_core, this change instead treats it as a non-breaking change taking the view that it will only break the `gfx_device_*` backends which can be updated and released alongside.

Version bumps:
* gfx_core `0.9` -> `0.9.1`
* gfx_device_gl `0.16` -> `0.16.1`
* gfx_device_dx11 `0.8.1` -> `0.8.2`
* gfx `0.18` -> `0.18.1`
* gfx_window_dxgi `0.19` -> `0.19.1`
* gfx_window_glfw `0.17` -> `0.17.1`
* gfx_window_glutin `0.30` -> `0.31`
* gfx_window_sdl `0.9` -> `0.9.1`

**All** these crates should be released to prevent external breakage.

Alternatively if it's desired to treat the changes as an external breakage this pr should be closed in favour of planning the release of v0.19.

Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
Co-authored-by: Michael Tang <tangmi@uw.edu>
Co-authored-by: Alex Butler <alexheretic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.