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

get HDR working #407

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/graphics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,19 @@ impl Error for ShaderError {
}
}

#[derive(Clone, PartialEq, Debug, Eq, Copy, Hash)]
/// A marker to prevent the creation of texture formats that aren't always safe to use in safe code without checks
pub struct PrivateConstruct(());

/// List of all the possible formats of input data when uploading to texture.
/// The list is built by intersection of texture formats supported by 3.3 core profile and webgl1.
#[repr(u8)]
#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
pub enum TextureFormat {
RGB8,
RGBA8,
/// Not every openGL version supported by Macroquad supports this texture format.
RGBA16(PrivateConstruct),
Depth,
Alpha,
}
Expand All @@ -314,10 +320,27 @@ impl TextureFormat {
match self {
TextureFormat::RGB8 => 3 * square,
TextureFormat::RGBA8 => 4 * square,
TextureFormat::RGBA16(_) => 4 * square,
TextureFormat::Depth => 2 * square,
TextureFormat::Alpha => 1 * square,
}
}
/// SAFETY:
/// Using this method on openGL versions that does not support RGBA16 textures can cause
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate on the UB it might cause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't know enough about openGL to answer that. All I know about it being UB to create textures with that format on OpenGL2 being UB is because of a comment on the Rust gamedev discord ( https://discord.com/channels/676678179678715904/677286494033018924/1145027947254792263 ).

But reading back I can see how I didn't word that nicely, as it isn't the method itself that can cause this, but then creating a texture with this format that is problematic on version 2 instead of 3. I'll reword it tomorrow to better reflect that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the comment to better reflect what is going on

/// UB
///
/// Make sure to check first before creating (render) textures with this format
pub unsafe fn rgba16_unchecked() -> Self {
Self::RGBA16(PrivateConstruct(()))
}
pub fn rgba16(context: &dyn RenderingBackend) -> Option<Self> {
if (!context.is_metal()) && !context.is_opengl3() {
return None;
}
///SAFETY: We checked before if the rendering backend supports rgba16 textures
let format = unsafe { Self::rgba16_unchecked() };
Some(format)
}
}

/// Sets the wrap parameter for texture.
Expand Down Expand Up @@ -1143,4 +1166,7 @@ pub trait RenderingBackend {
/// NOTE: num_instances > 1 might be not supported by the GPU (gl2.1 and gles2).
/// `features.instancing` check is required.
fn draw(&self, base_element: i32, num_elements: i32, num_instances: i32);

fn is_opengl3(&self) -> bool;
fn is_metal(&self) -> bool;
}
12 changes: 12 additions & 0 deletions src/graphics/gl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl From<TextureFormat> for (GLenum, GLenum, GLenum) {
match format {
TextureFormat::RGB8 => (GL_RGB, GL_RGB, GL_UNSIGNED_BYTE),
TextureFormat::RGBA8 => (GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE),
TextureFormat::RGBA16(_) => (GL_RGBA16F, GL_RGBA, GL_UNSIGNED_BYTE),
TextureFormat::Depth => (GL_DEPTH_COMPONENT, GL_DEPTH_COMPONENT, GL_UNSIGNED_SHORT),
#[cfg(target_arch = "wasm32")]
TextureFormat::Alpha => (GL_ALPHA, GL_ALPHA, GL_UNSIGNED_BYTE),
Expand Down Expand Up @@ -1331,4 +1332,15 @@ impl RenderingBackend for GlContext {
);
}
}

fn is_opengl3(&self) -> bool {
///SAFETY:
/// the creation of this context already calls is_gl2
/// So, presumably calling it here is fine as well?
!unsafe { is_gl2() }
}

fn is_metal(&self) -> bool {
false
}
}
6 changes: 6 additions & 0 deletions src/graphics/metal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1116,4 +1116,10 @@ impl RenderingBackend for MetalContext {
self.current_frame_index = 0;
}
}
fn is_metal(&self) -> bool {
true
}
fn is_opengl3(&self) -> bool {
false
}
}
Loading