diff --git a/crates/asset_material/src/material_catalog.rs b/crates/asset_material/src/material_catalog.rs index 2f5cfc5..71e3a61 100644 --- a/crates/asset_material/src/material_catalog.rs +++ b/crates/asset_material/src/material_catalog.rs @@ -19,20 +19,29 @@ pub const TS_T5_COLOR0_MAP: u8 = 0x0C; pub const TS_T5_COLOR15_MAP: u8 = 0x1B; pub const TS_T5_THROW_MAP: u8 = 0x1C; -/// Which prepared variant a decoded image is: the source bytes it came from -/// and the recipe applied to them. +/// Which prepared variant a decoded image is: the bytes it decodes to, and the +/// image built around those bytes. /// /// Two claims on the same *name* are not the same image — IW4, IW5 and T5 each /// ship their own `hud_teamcaret` — so a name is not enough to say whether one /// plan's decode could have answered another's. This is, and it travels on the /// row so the merge can say which of the two happened to a claim it dropped: /// the same bytes prepared twice, or a genuine override by another source. +/// +/// The two halves are separate because only one of them is the decode. The +/// payload is what `decode` reads and produces; `usage` is everything the +/// decoded bytes are then wrapped in. A claim that matches on `payload` and +/// differs on `usage` wanted the very same texels — it is repeated work, and +/// the census has to be able to say so rather than call it another source. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct ImageVariantId { - /// Digest of the resolved archive entries the decode read, in order. - pub source: u64, - /// The decode options that change the prepared payload. - pub recipe: u32, + /// Digest of everything that decides a byte of the decoded payload: the + /// resolved archive entries the decode reads, in order, and the map type, + /// which chooses between the 2D and the cubemap decode. + pub payload: u64, + /// The options that change no byte of the payload but do change the image + /// wrapped around it: the view's colour space and the sampler. + pub usage: u32, } #[derive(Clone, Debug)] diff --git a/crates/asset_material/src/material_images.rs b/crates/asset_material/src/material_images.rs index c6d81fa..63014ed 100644 --- a/crates/asset_material/src/material_images.rs +++ b/crates/asset_material/src/material_images.rs @@ -676,13 +676,18 @@ fn decode_requests_in_parallel( .collect() } -/// What a decode reads and what it does to it, as one identity. +/// What a decode reads and what is built around the result, as one identity. /// -/// The source half is the ordered list of archive entries the decode would +/// The payload half is the ordered list of archive entries the decode would /// try, by CRC, size and name — not the image's name, which three games spell -/// the same and fill differently. The recipe half is every option that changes -/// the prepared payload: two rows that want the same entry as a normal map and -/// as a colour map are two variants and must not share a buffer. +/// the same and fill differently — plus the map type, because a cubemap and a +/// 2D image read the same entry into different bytes. +/// +/// The usage half is the rest, and none of it reaches `image.data`: a colour +/// space picks `Rgba8Unorm` over `Rgba8UnormSrgb` for the same texels, and a +/// sampler is not a property of the image at all. Keeping it in the identity +/// is what lets the merge tell "the same texels, wanted twice" from "another +/// game shipped a different image under this name". fn variant_of( index: &IwdIndex, source: &AuthoredImage, @@ -690,22 +695,19 @@ fn variant_of( (sampler_state, is_normal, alpha_test_color, force_linear): (u8, bool, bool, bool), ) -> Option { let candidates = index.0.image_candidates(name)?; - let mut digest = crate::fnv1a64(name.as_bytes()); + let mut payload = crate::fnv1a64(name.as_bytes()); for candidate in candidates { - digest = crate::fnv1a64_more(digest, &candidate.crc32().to_le_bytes()); - digest = crate::fnv1a64_more(digest, &candidate.size().to_le_bytes()); - digest = crate::fnv1a64_more(digest, candidate.entry().as_bytes()); + payload = crate::fnv1a64_more(payload, &candidate.crc32().to_le_bytes()); + payload = crate::fnv1a64_more(payload, &candidate.size().to_le_bytes()); + payload = crate::fnv1a64_more(payload, candidate.entry().as_bytes()); } - let recipe = u32::from(sampler_state) + payload = crate::fnv1a64_more(payload, &[source.map_type]); + let usage = u32::from(sampler_state) | u32::from(is_normal) << 8 | u32::from(alpha_test_color) << 9 | u32::from(force_linear) << 10 - | u32::from(source.use_srgb_reads) << 11 - | u32::from(source.map_type) << 12; - Some(ImageVariantId { - source: digest, - recipe, - }) + | u32::from(source.use_srgb_reads) << 11; + Some(ImageVariantId { payload, usage }) } fn decode_one_request( @@ -1879,12 +1881,20 @@ pub struct ImageMergeCensus { pub shared_bytes: u64, pub shared_variants: usize, /// Of the discarded variants, how many the winning row had prepared from - /// the *same* source and recipe — the same bytes twice — against how many - /// were genuinely answered by a different source, and how many the merge + /// the identical variant — the same bytes twice — against how many were + /// genuinely answered by a different source, and how many the merge /// answered from an inline body. pub discarded_same_variant: usize, pub discarded_overridden: usize, pub discarded_inline: usize, + /// Of the discards the census would otherwise call overridden, how many + /// decoded the *same payload* as the winner and differed only in the + /// colour space or the sampler wrapped around it: duplicate decodes that + /// sharing cannot catch while the wrapping lives inside the `Image`. + pub discarded_same_payload: usize, + /// A discarded name whose winner holds the same payload under a different + /// wrapping. + pub first_same_payload: Option, /// Of the same-variant discards, how many cost nothing because the buffer /// was shared rather than decoded twice. pub discarded_shared: usize, @@ -1909,16 +1919,20 @@ impl ImageMergeCensus { .collect::>() .join(" "); Some(format!( - "{} variants, {} bytes ({} same source+recipe of which {} cost nothing by sharing, {} overridden by another source, {} by an inline body): {reasons}{}{}", + "{} variants, {} bytes ({} same variant of which {} cost nothing by sharing, {} same payload under another wrapping, {} overridden by another source, {} by an inline body): {reasons}{}{}{}", self.discarded_variants, self.discarded_decoded_bytes, self.discarded_same_variant, self.discarded_shared, + self.discarded_same_payload, self.discarded_overridden, self.discarded_inline, self.first_discarded .as_ref() .map_or_else(String::new, |name| format!(" first={name}")), + self.first_same_payload + .as_ref() + .map_or_else(String::new, |name| format!(" first_same_payload={name}")), self.first_override .as_ref() .map_or_else(String::new, |name| format!(" first_override={name}")), @@ -2257,6 +2271,12 @@ impl DecodedImageBatch { census.discarded_shared += 1; } } + Winner::SamePayload => { + census.discarded_same_payload += 1; + census + .first_same_payload + .get_or_insert_with(|| name.clone()); + } Winner::OtherSource => { census.discarded_overridden += 1; census.first_override.get_or_insert(name); @@ -2281,9 +2301,15 @@ impl DecodedImageBatch { /// images under one name, where preparing both and keeping one is a /// *scheduling* question and not a duplicate at all. enum Winner { - /// The kept row holds the same source and recipe this plan prepared. + /// The kept row holds the identical variant this plan prepared. SameVariant, - /// The kept row holds a different variant: another source answered. + /// The kept row decoded the same payload and wrapped it differently: the + /// same texels were read twice, and only the colour space or the sampler + /// told the two claims apart. Avoidable work that sharing cannot take, + /// because the buffer that would be shared is inside an `Image` that + /// carries the wrapping with it. + SamePayload, + /// The kept row holds a different payload: another source answered. OtherSource, /// The kept row carries its own inline body. InlineBody, @@ -2304,6 +2330,7 @@ fn winner_of(catalog: &MaterialDefinitions, name: &str, ours: Option Winner::SameVariant, + (Some(theirs), Some(ours)) if theirs.payload == ours.payload => Winner::SamePayload, (Some(_), Some(_)) => Winner::OtherSource, _ => Winner::Unknown, } @@ -2447,10 +2474,10 @@ mod tests { } #[test] - fn the_same_source_and_recipe_is_prepared_once() { + fn the_same_payload_and_usage_is_prepared_once() { let variant = ImageVariantId { - source: 0xfeed_face, - recipe: 7, + payload: 0xfeed_face, + usage: 7, }; let first = share_or_decode(variant, 0, one_pixel); let first = arc_of(first); @@ -2472,19 +2499,21 @@ mod tests { } #[test] - fn a_different_recipe_over_the_same_source_is_a_different_variant() { - // Same archive entry wanted as a colour map and as a normal map: two - // prepared payloads, and sharing one between them would be wrong. - let source = 0x1234_5678; + fn a_different_usage_over_the_same_payload_is_a_different_variant() { + // Same texels wanted as a colour map and as a normal map. The bytes + // are identical; the `Image` built around them is not, because it + // carries the colour space, so the two cannot share one buffer while + // the wrapping lives inside it. + let payload = 0x1234_5678; let colour = arc_of(share_or_decode( - ImageVariantId { source, recipe: 0 }, + ImageVariantId { payload, usage: 0 }, 0, one_pixel, )); let normal = arc_of(share_or_decode( ImageVariantId { - source, - recipe: 1 << 8, + payload, + usage: 1 << 8, }, 0, one_pixel, @@ -2492,13 +2521,67 @@ mod tests { assert!(!Arc::ptr_eq(&colour, &normal)); } + fn undecoded_row(name: &str) -> AuthoredImage { + AuthoredImage { + name: crate::AssetRef::decode(name), + map_type: 0, + semantic: 0, + category: 0, + use_srgb_reads: false, + width: 1, + height: 1, + depth: 1, + level_count: 1, + format: 21, + payload: Vec::new(), + decoded: None, + decoded_variant: None, + pending_decode: None, + } + } + + #[test] + fn a_winner_holding_the_same_texels_is_not_an_override() { + // The sampler and the colour space are not the image: this runtime's + // texture table binds a sampler state beside a texture, not inside it. + // So two claims that differ only there read the same archive entry into + // the same bytes, and calling that an override hides a duplicate decode + // behind a number that says there is nothing to share. + let mut catalog = MaterialDefinitions::default(); + let mut kept = undecoded_row("~-white"); + kept.decoded = Some(Image::default()); + kept.decoded_variant = Some(ImageVariantId { + payload: 11, + usage: 0, + }); + catalog.images.push(kept); + + let ours = Some(ImageVariantId { + payload: 11, + usage: 1 << 8, + }); + assert!(matches!( + winner_of(&catalog, "~-white", ours), + Winner::SamePayload + )); + + let other = Some(ImageVariantId { + payload: 22, + usage: 0, + }); + assert!(matches!( + winner_of(&catalog, "~-white", other), + Winner::OtherSource + )); + } + #[test] fn a_variant_nobody_holds_any_more_is_decoded_again() { // The registry keeps weak handles, so the corpus does not stay resident // for the life of the process once the batches holding it are gone. let variant = ImageVariantId { - source: 0xdead_beef, - recipe: 0, + payload: 0xdead_beef, + usage: 0, }; drop(arc_of(share_or_decode(variant, 0, one_pixel))); let again = share_or_decode(variant, 0, one_pixel); @@ -2513,8 +2596,8 @@ mod tests { #[test] fn a_decode_that_fails_does_not_park_the_next_asker() { let variant = ImageVariantId { - source: 0x0bad_0bad, - recipe: 0, + payload: 0x0bad_0bad, + usage: 0, }; let gap = share_or_decode(variant, 0, || ImageOutcome::Missing { gap: "absent".to_owned(), diff --git a/crates/assets/src/session_load.rs b/crates/assets/src/session_load.rs index bd7a264..76e521c 100644 --- a/crates/assets/src/session_load.rs +++ b/crates/assets/src/session_load.rs @@ -872,7 +872,7 @@ pub async fn load_prepared_match( census.filled_rows, census.already_decoded, census.discarded_variants, )); report.push(format!( - "{label} image demand: claimed_rows={} canonical_variants={} prepared_variants={} duplicate_claims={} final_cpu_bytes={} produced_bytes={} shared_variants={} shared_bytes={} discarded_decoded_bytes={}", + "{label} image demand: claimed_rows={} canonical_variants={} prepared_variants={} duplicate_claims={} final_cpu_bytes={} produced_bytes={} shared_variants={} shared_bytes={} discarded_decoded_bytes={} discarded_same_payload={}", census.claimed_rows, census.canonical_variants, census.prepared_variants, @@ -882,6 +882,7 @@ pub async fn load_prepared_match( census.shared_variants, census.shared_bytes, census.discarded_decoded_bytes, + census.discarded_same_payload, )); if let Some(line) = census.discard_line() { report.push(format!("{label} image discard: {line}")); diff --git a/crates/bootstrap/src/bench/load_report.rs b/crates/bootstrap/src/bench/load_report.rs index 64c5ec2..579684c 100644 --- a/crates/bootstrap/src/bench/load_report.rs +++ b/crates/bootstrap/src/bench/load_report.rs @@ -100,7 +100,7 @@ fn image_plans(out: &mut Vec) { let (shared, shared_bytes) = assets::shared_variant_census(); if shared > 0 { out.push(format!( - " {shared} prepared variants ({}) were answered out of another plan's work instead of being decoded a second time — same archive entry, same recipe.", + " {shared} prepared variants ({}) were answered out of another plan's work instead of being decoded a second time — same archive entry, same variant.", mib(shared_bytes), )); } else { diff --git a/docs/MAP-LOAD.md b/docs/MAP-LOAD.md index 9974fd7..c03322f 100644 --- a/docs/MAP-LOAD.md +++ b/docs/MAP-LOAD.md @@ -27,8 +27,8 @@ goes **before** the map — its techset tables are needed before (world, one material population, clip, weapons, catalogs and `PreparedMap`). A walk enqueues its own image plan the moment it is ready, and the catalog keeps -one row per image name. Two plans resolving the *same* archive entry with the -same recipe share one decode; a name three games spell alike but fill +one row per image name. Two plans resolving the *same* archive entry into the +same payload share one decode; a name three games spell alike but fill differently is an override, not a duplicate, and `load_jobs.csv` ([`BENCH.md`](BENCH.md)) counts the two apart. The per-game adapter is `assets::lane` (`ZoneGame` → iw4/iw5/t5); a lane gap is a typed `LaneGap`, never