Skip to content

Commit a70b3d5

Browse files
committed
fix(resolve): follow extension-to-extension dependencies
Parse the `requires=` (GL) and `depends=` (Vulkan) attributes on <extension> elements, which declare prerequisite extensions. Previously these attributes were silently discarded during XML parsing. ir.rs: add `depends: Vec<String>` to RawExtension. parse/features.rs: new parse_extension_depends() extracts extension name tokens from both GL's comma-separated format and Vulkan's boolean expression format (splits on comma, plus, parentheses). Version tokens like VK_VERSION_1_1 are included harmlessly — the resolver skips them since they don't match any extension name. resolve.rs: new SelectionReason::Dependency variant and fixed-point dependency-following loop in select_extensions, running after the initial filter but before --promoted/--predecessors. Transitively pulls in prerequisite extensions not already selected. preamble.rs: dependency extensions shown in the summary line and listed by name alongside promoted/predecessor extensions. Note: GL extension dependencies are largely absent from gl.xml (the information exists only in spec prose text, not in machine- readable attributes). This is a known limitation of the GL XML registry. Vulkan's `depends=` attribute is more consistently populated. Signed-off-by: Steven Noonan <steven@uplinklabs.net>
1 parent 29e33b9 commit a70b3d5

4 files changed

Lines changed: 126 additions & 3 deletions

File tree

src/ir.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ pub struct RawExtension {
203203
pub protect: Vec<String>,
204204
/// Extension registry number, used for enum offset calculation.
205205
pub number: Option<u32>,
206+
/// Extensions this one depends on (from `requires=` or `depends=` attribute).
207+
/// All referenced extension names, regardless of AND/OR semantics in the
208+
/// original attribute — if a name appears at all, it's a prerequisite the
209+
/// loader should include.
210+
pub depends: Vec<String>,
206211
}
207212

208213
// ---------------------------------------------------------------------------

src/parse/features.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,13 @@ fn parse_extensions(
202202

203203
let number = node.attribute("number").and_then(|s| s.parse().ok());
204204

205+
// Extension-to-extension dependencies: GL uses `requires=` (comma-
206+
// separated), Vulkan uses `depends=` with `+` (AND), `,` (OR), and
207+
// parentheses. We extract every extension-name-looking token from
208+
// whichever attribute is present — the resolver needs all prerequisites
209+
// regardless of AND/OR semantics.
210+
let depends = parse_extension_depends(node);
211+
205212
let requires = node
206213
.children()
207214
.filter(|n| n.is_element() && n.tag_name().name() == "require")
@@ -214,6 +221,7 @@ fn parse_extensions(
214221
requires,
215222
protect,
216223
number,
224+
depends,
217225
});
218226
}
219227

@@ -231,6 +239,37 @@ fn parse_extensions(
231239

232240
Ok(extensions)
233241
}
242+
// ---------------------------------------------------------------------------
243+
// Parse extension dependency attributes
244+
// ---------------------------------------------------------------------------
245+
246+
/// Extract extension dependency names from the `requires=` (GL) or `depends=`
247+
/// (Vulkan) attribute on an `<extension>` element.
248+
///
249+
/// GL uses comma-separated names: `requires="GL_ARB_draw_indirect"`
250+
/// Vulkan uses a boolean expression: `depends="VK_KHR_foo+VK_KHR_bar,VK_VERSION_1_1"`
251+
/// with `+` (AND), `,` (OR), and parentheses.
252+
///
253+
/// We split on all delimiters and return every token that looks like an
254+
/// extension name (contains `_` and doesn't start with a digit). Version
255+
/// requirements like `VK_VERSION_1_1` are included — the resolver filters
256+
/// them against the actual extension list.
257+
fn parse_extension_depends(node: roxmltree::Node<'_, '_>) -> Vec<String> {
258+
let attr = node
259+
.attribute("depends")
260+
.or_else(|| node.attribute("requires"));
261+
262+
let Some(raw) = attr else {
263+
return Vec::new();
264+
};
265+
266+
raw.split(|c: char| c == ',' || c == '+' || c == '(' || c == ')')
267+
.map(str::trim)
268+
.filter(|s| !s.is_empty() && s.contains('_'))
269+
.map(str::to_string)
270+
.collect()
271+
}
272+
234273
// ---------------------------------------------------------------------------
235274
// Parse <require> and <remove> blocks
236275
// ---------------------------------------------------------------------------

src/preamble.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ pub fn build_preamble(fs: &FeatureSet, command_line: &str) -> String {
3030
lines.push(" *".to_string());
3131
lines.push(format!(" * {}", extension_summary(fs)));
3232

33-
// List implicitly-included extensions (promoted, predecessor) so the
34-
// user can see what was pulled in automatically.
33+
// List implicitly-included extensions (dependency, promoted, predecessor)
34+
// so the user can see what was pulled in automatically.
3535
for (label, reason) in &[
36+
("dependency", SelectionReason::Dependency),
3637
("promoted", SelectionReason::Promoted),
3738
("predecessor", SelectionReason::Predecessor),
3839
] {
@@ -50,7 +51,7 @@ pub fn build_preamble(fs: &FeatureSet, command_line: &str) -> String {
5051

5152
// ---- gloam license ----
5253
lines.push(" *".to_string());
53-
lines.push(format!(" * gloam Copyright (c) {year} Steven Noonan"));
54+
lines.push(format!(" * Copyright (c) {year} Steven Noonan"));
5455
lines.push(" * SPDX-License-Identifier: MIT".to_string());
5556

5657
// ---- Khronos ----
@@ -93,6 +94,7 @@ fn extension_summary(fs: &FeatureSet) -> String {
9394
let n_all = count(SelectionReason::AllExtensions);
9495
let n_explicit = count(SelectionReason::Explicit);
9596
let n_mandatory = count(SelectionReason::Mandatory);
97+
let n_dependency = count(SelectionReason::Dependency);
9698
let n_promoted = count(SelectionReason::Promoted);
9799
let n_predecessor = count(SelectionReason::Predecessor);
98100

@@ -108,6 +110,9 @@ fn extension_summary(fs: &FeatureSet) -> String {
108110
if n_mandatory > 0 {
109111
parts.push(format!("{n_mandatory} mandatory"));
110112
}
113+
if n_dependency > 0 {
114+
parts.push(format!("{n_dependency} dependency"));
115+
}
111116
if n_promoted > 0 {
112117
parts.push(format!("{n_promoted} promoted"));
113118
}
@@ -316,6 +321,30 @@ mod tests {
316321
assert!(p.contains("predecessor: GL_ARB_parallel_shader_compile"));
317322
}
318323

324+
#[test]
325+
fn preamble_lists_dependency_extensions() {
326+
let mut fs = stub_fs("gl");
327+
fs.extensions = vec![
328+
stub_ext("GL_ARB_multi_draw_indirect", SelectionReason::Explicit),
329+
stub_ext("GL_ARB_draw_indirect", SelectionReason::Dependency),
330+
];
331+
let p = build_preamble(&fs, "gloam --api gl:core=3.3 c");
332+
assert!(p.contains("dependency: GL_ARB_draw_indirect"));
333+
}
334+
335+
#[test]
336+
fn summary_with_dependencies() {
337+
let mut fs = stub_fs("gl");
338+
fs.extensions = vec![
339+
stub_ext("GL_ARB_multi_draw_indirect", SelectionReason::Explicit),
340+
stub_ext("GL_ARB_draw_indirect", SelectionReason::Dependency),
341+
];
342+
assert_eq!(
343+
extension_summary(&fs),
344+
"Extensions: 1 explicit, 1 dependency (2 total)"
345+
);
346+
}
347+
319348
#[test]
320349
fn preamble_no_extension_section_when_empty() {
321350
let fs = stub_fs("vk");

src/resolve.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,9 @@ pub enum SelectionReason {
707707
AllExtensions,
708708
/// WGL mandatory extensions (always required for WGL to function).
709709
Mandatory,
710+
/// Auto-included because a selected extension declares it as a dependency
711+
/// (via the `requires=` or `depends=` XML attribute).
712+
Dependency,
710713
/// Auto-included because its commands were promoted into a requested core version.
711714
Promoted,
712715
/// Auto-included as a predecessor of an already-selected extension.
@@ -777,6 +780,53 @@ fn select_extensions<'a>(
777780
})
778781
.collect();
779782

783+
// Build an extension name → index lookup for dependency resolution.
784+
let ext_by_name: HashMap<&str, usize> = raw
785+
.extensions
786+
.iter()
787+
.enumerate()
788+
.map(|(i, e)| (e.name.as_str(), i))
789+
.collect();
790+
791+
// Dependency-following pass: walk the `depends` field of each selected
792+
// extension and pull in any prerequisite extensions not already selected.
793+
// Fixed-point loop because dependencies can be transitive — pulling in
794+
// extension A may require extension B which requires extension C.
795+
// Runs before --promoted and --predecessors so that dependency-pulled
796+
// extensions' commands are visible to those passes.
797+
loop {
798+
let already: HashSet<&str> = selected.iter().map(|e| e.raw.name.as_str()).collect();
799+
let prev_len = selected.len();
800+
801+
// Collect unique dependency names from all currently selected extensions.
802+
let needed: HashSet<&str> = selected
803+
.iter()
804+
.flat_map(|e| e.raw.depends.iter().map(String::as_str))
805+
.filter(|dep| {
806+
!already.contains(dep)
807+
&& ext_by_name.contains_key(dep)
808+
// Only pull in extensions that support a requested API.
809+
&& raw.extensions[ext_by_name[dep]]
810+
.supported
811+
.iter()
812+
.any(|s| api_set.contains(s.as_str()))
813+
})
814+
.collect();
815+
816+
for dep_name in needed {
817+
if let Some(&idx) = ext_by_name.get(dep_name) {
818+
selected.push(SelectedExt {
819+
raw: &raw.extensions[idx],
820+
reason: SelectionReason::Dependency,
821+
});
822+
}
823+
}
824+
825+
if selected.len() == prev_len {
826+
break;
827+
}
828+
}
829+
780830
// Build the bidirectional alias maps once — they're used by both the
781831
// --promoted and --predecessors passes.
782832
let cmd_to_alias: HashMap<&str, &str> = if want_promoted || want_predecessors {

0 commit comments

Comments
 (0)