Skip to content

Some proposals - #7

Draft
feliwir wants to merge 9 commits into
darksylinc:masterfrom
feliwir:use-gli
Draft

Some proposals#7
feliwir wants to merge 9 commits into
darksylinc:masterfrom
feliwir:use-gli

Conversation

@feliwir

@feliwir feliwir commented Oct 5, 2021

Copy link
Copy Markdown

This is more of a meta PR, asking for changes that make sense to you / you'd accept a PR for:

  • Use gli library for universal texture formats that can be translated to GL/VK & DirectX (making DDS support much more viable). See https://github.com/g-truc/gli
  • Replace FreeImage with single file stb_image, which has similar support for texture formats and is a much leaner dependency
  • Replace SDL2 with GLFW, again since it's a much leaner dependency and we don't require any audio / input features from SDL2. Also don't show the window
  • Restructure library & folder layout, so it's easier to add a VK or CPU backend

@darksylinc

darksylinc commented Oct 5, 2021

Copy link
Copy Markdown
Owner

OK this will take some time to review, a few comments:

  1. I'm a bit hesitant about FreeImage removal. stb_image was explicitly rejected because it was written for video game engines in mind (meaning: if the file doesn't open with stb_image then you save it again with different setting or extension). std_image won't handle all variations (e.g. like interlaced JPG). On the other hand, it's leaner and worth evaluating if the user shouldn't just re-export the texture instead of expecting betsy to accept anything you throw at it
  2. Your PR overwrites .clang-format, the final PR must not.
  3. GLFW: If it's readable, leaner and works then great!
  4. GLI abstraction: Sounds cool!!! at least in theory.
    • Ok I took a look at the commit and right now it's only mostly swapping our enums with GLI's, therefore I don't understand yet what GLI brings to the table.

Betsy was commissioned for Godot, thus the way the code was structured was to maximize easiness of integration into other projects, which is why the classes have abstracted GL code away. A key aspect we wish to conserve is that integration (aka copy paste into your own project) should be straightforward. As long as this aspect is preserved I'm cool with it. I wanted to go with Vulkan but time was limited and Vulkan initial setup is harder and we were on a tight budget; which is why we went for OpenGL (they both support GLSL anyway, and 95% of the project is GLSL code)

@feliwir

feliwir commented Oct 7, 2021

Copy link
Copy Markdown
Author

So to answer some questions:

  1. I didn't like the way we pulled some sources from FreeImage into this repository. We should either use a real FreeImage package with find_package or do something else.
  2. Yea, that was for my fork. I'd remove that and make a separate PR for each feature.
  3. good
  4. GLI has the advantage that it can also translate to DirectX and Vulkan formats. As you probably know best yourself the KTX format does store the format parameters in OpenGL internal format specifiers. But e.g. DDS does use the internal DirectX format. So e.g. if we wanted to support DDS (which i definetly want to add aswell, since it's still the most common for compressed textures we need to be able to translate to that aswell. Another benefit of GLI is the mentioned Vulkan support, which it can translate to aswell. It looks like that:
	gli::gl GL(gli::gl::PROFILE_GL33);
	gli::gl::format const GLFormat = GL.translate(Texture.format(), Texture.swizzles());
       	gli::dx DX();
       	gli::dx::format const DXFormat = DX.translate(Texture.format(), Texture.swizzles());

(and so on for Vulkan)
5. Another point i forgot: I compiled the shaders into the binary and did the preprocessing with cmake. So this should reduce the headache of requiring the shaders next to the binary and also reduce startup time, since no shader preprocessing & file io is performed.

@darksylinc

Copy link
Copy Markdown
Owner

GLI has the advantage that it can also translate to DirectX and Vulkan formats. As you probably know best yourself the KTX format does store the format parameters in OpenGL internal format specifiers. But e.g. DDS does use the internal DirectX format. So e.g. if we wanted to support DDS (which i definetly want to add aswell, since it's still the most common for compressed textures we need to be able to translate to that aswell. Another benefit of GLI is the mentioned Vulkan support, which it can translate to aswell. It looks like that:

Oh I see where it's going. More context:

  • PixelFormat maps 1:1 to DXGI_FORMAT and VK_FORMAT, e.g. PFG_RGBA16_FLOAT is DXGI_FORMAT_R32G32B32_FLOAT and VK_FORMAT_R16G16B16A16_SFLOAT. You can find the mappings already in Ogre's database which is where I grabbed from (Vulkan, D3D11). It has a lot more formats than what Betsy needs though
  • DDS comes in two flavours:
    1. Pre-DX10 header, where BC1-BC5 is done through FOURCC codes (using DXT1-5 nomenclature, note DXT1-5 do not match to BC1-BC5, i.e. BC5 is not DXT5) and for RGBA modes uses esoteric flags that are a PITA to parse.
    2. DX10 header, which uses DXGI formats

If we're going to use GLI to support Pre-DX10 headers then it makes sense (assuming GLI supports pre-DX10 dds). Otherwise, integrating a large library like GLI just for format conversion sounds overkill when it's only a large switch statement per backend.

  1. Another point i forgot: I compiled the shaders into the binary and did the preprocessing with cmake. So this should reduce the headache of requiring the shaders next to the binary and also reduce startup time, since no shader preprocessing & file io is performed.

OK that sounds very useful! As long as it can be toggled on/off at build time (both for development and for platforms where embedding the files into the exe can prove difficult)

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.

2 participants