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

glsl-out: Implement bounds checks for ImageLoad #1889

Merged
merged 3 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
))?,
&params.glsl,
&pipeline_options,
params.bounds_check_policies,
)
.unwrap_pretty();
writer.write()?;
Expand Down
83 changes: 71 additions & 12 deletions src/back/glsl/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ bitflags::bitflags! {
const FMA = 1 << 18;
/// Texture samples query
const TEXTURE_SAMPLES = 1 << 19;
/// Texture levels query
const TEXTURE_LEVELS = 1 << 20;
/// Image size query
const IMAGE_SIZE = 1 << 21;
}
}

Expand Down Expand Up @@ -104,9 +108,11 @@ impl FeaturesManager {
check_feature!(DYNAMIC_ARRAY_SIZE, 430, 310);
check_feature!(MULTI_VIEW, 140, 310);
// Only available on glsl core, this means that opengl es can't query the number
// of samples in a image and neither do bound checks on the sample argument
// of texelFecth
// of samples nor levels in a image and neither do bound checks on the sample nor
// the level argument of texelFecth
check_feature!(TEXTURE_SAMPLES, 150);
check_feature!(TEXTURE_LEVELS, 130);
check_feature!(IMAGE_SIZE, 430, 310);

// Return an error if there are missing features
if missing.is_empty() {
Expand Down Expand Up @@ -223,6 +229,11 @@ impl FeaturesManager {
)?;
}

if self.0.contains(Features::TEXTURE_LEVELS) && version < Version::Desktop(430) {
// https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_texture_query_levels.txt
writeln!(out, "#extension GL_ARB_texture_query_levels : require")?;
}

Ok(())
}
}
Expand Down Expand Up @@ -376,27 +387,75 @@ impl<'a, W> Writer<'a, W> {
}
}

// Loop trough all expressions in both functions and entry points
// We will need to pass some of the members to a closure, so we need
// to separate them otherwise the borrow checker will complain, this
// shouldn't be needed in rust 2021
let &mut Self {
module,
info,
ref mut features,
entry_point,
entry_point_idx,
ref policies,
..
} = self;

// Loop trough all expressions in both functions and the entry point
// to check for needed features
for (_, expr) in self
.module
for (expressions, info) in module
.functions
.iter()
.flat_map(|(_, f)| f.expressions.iter())
.chain(self.entry_point.function.expressions.iter())
.map(|(h, f)| (&f.expressions, &info[h]))
.chain(std::iter::once((
&entry_point.function.expressions,
info.get_entry_point(entry_point_idx as usize),
)))
{
match *expr {
for (_, expr) in expressions.iter() {
match *expr {
// Check for fused multiply add use
Expression::Math { fun, .. } if fun == MathFunction::Fma => {
self.features.request(Features::FMA)
features.request(Features::FMA)
}
// Check for samples query
// Check for queries that neeed aditonal features
Expression::ImageQuery {
query: crate::ImageQuery::NumSamples,
image,
query,
..
} => self.features.request(Features::TEXTURE_SAMPLES),
} => match query {
// Storage images use `imageSize` which is only available
// in glsl > 420
//
// layers queries are also implemented as size queries
crate::ImageQuery::Size { .. } | crate::ImageQuery::NumLayers => {
jimblandy marked this conversation as resolved.
Show resolved Hide resolved
if let TypeInner::Image {
class: crate::ImageClass::Storage { .. }, ..
} = *info[image].ty.inner_with(&module.types) {
features.request(Features::IMAGE_SIZE)
}
},
crate::ImageQuery::NumLevels => features.request(Features::TEXTURE_LEVELS),
crate::ImageQuery::NumSamples => features.request(Features::TEXTURE_SAMPLES),
}
,
// Check for image loads that needs bound checking on the sample
// or level argument since this requires a feature
Expression::ImageLoad {
sample, level, ..
} => {
if policies.image != crate::proc::BoundsCheckPolicy::Unchecked {
if sample.is_some() {
features.request(Features::TEXTURE_SAMPLES)
}

if level.is_some() {
features.request(Features::TEXTURE_LEVELS)
}
}
}
_ => {}
}
}
}

self.features.check_availability(self.options.version)
Expand Down
Loading