Merge pull request #9212 from horizon-rust/fix/8669-useitemon-freeze

🤖 Agent: The freeze came from map searches opened from chests, which built full jigsaw starts and scanned every chunk of each saved region for each candidate. Same answers on main and this branch (24 targets × 5 points × 4 runs); on a pregenerated world the bamboo-camp map search drops from 2.1–3.6 s to 0.04–0.07 s of server-thread CPU, and main hit the 60 s watchdog in 4 of 9 runs. fmt, both clippies and the touched crates' tests pass, plus 10 harnesses incl. server_world_gen (/locate against vanilla); dedicated_server's vanilla oracle crashed on the loaded desktop, so CI runs it.

(cherry picked from commit 6d43a3593b)

crates/horizon-client-render/src/spans.rs

@@ -68,27 +68,48 @@ fn cpu_if_enabled() -> Option<Duration> {
 /// Run `f`, recording its duration under `name` when spans are enabled.
 #[inline]
 pub fn time<T>(name: &'static str, f: impl FnOnce() -> T) -> T {
+    let _span = guard(name);
+    f()
+}
+/// A span that ends where it is dropped: [`time`] for a step with several ways out (early
+/// returns). Nothing is read while spans are off.
+#[must_use = "the span ends when the guard is dropped"]
+pub struct Guard {
+    open: Option<(&'static str, Instant, Option<Duration>, u8)>,
+    _heap: Option<horizon_threads::heap_census::Tag>,
+}
+/// Start a span named `name` that ends when the returned [`Guard`] is dropped.
+#[inline]
+pub fn guard(name: &'static str) -> Guard {
     let _heap = horizon_threads::heap_tag(name);
     if !recording_here() {
-        return f();
+        return Guard { open: None, _heap };
     }
     let depth = DEPTH.with(|d| d.replace(d.get().saturating_add(1)));
-    let (started, cpu) = (Instant::now(), cpu_if_enabled());
-    let out = f();
-    let ms = started.elapsed().as_secs_f32() * 1000.0;
-    let cpu_ms = cpu
-        .and_then(|start| Some((thread_cpu()?, start)))
-        .map_or(ms, |(end, start)| (end - start).as_secs_f32() * 1000.0);
-    DEPTH.with(|d| d.set(depth));
-    SPANS.with(|s| {
-        s.borrow_mut().push(Span {
-            name,
-            ms,
-            cpu_ms,
-            depth,
-        })
-    });
-    out
+    Guard {
+        open: Some((name, Instant::now(), cpu_if_enabled(), depth)),
+        _heap,
+    }
+}
+impl Drop for Guard {
+    fn drop(&mut self) {
+        let Some((name, started, cpu, depth)) = self.open.take() else {
+            return;
+        };
+        let ms = started.elapsed().as_secs_f32() * 1000.0;
+        let cpu_ms = cpu
+            .and_then(|start| Some((thread_cpu()?, start)))
+            .map_or(ms, |(end, start)| (end - start).as_secs_f32() * 1000.0);
+        DEPTH.with(|d| d.set(depth));
+        SPANS.with(|s| {
+            s.borrow_mut().push(Span {
+                name,
+                ms,
+                cpu_ms,
+                depth,
+            })
+        });
+    }
 }
 /// Notes that one step inside a span took `ms` milliseconds doing `what` (one player's
 /// packets, say), when this thread records spans and the step took at least [`NOTE_MS`]: a
@@ -551,6 +572,42 @@ mod tests {
         .unwrap();
     }
 
+    /// A guarded span ends on every way out of its step, an early return's too, and holds the
+    /// spans that ran inside it until then; the next span outside it is back at its depth.
+    #[test]
+    fn a_guarded_span_ends_where_its_step_returns() {
+        fn step(early: bool) -> u32 {
+            let _span = super::guard("use_block");
+            super::time("open_loot", || ());
+            if early {
+                return 1;
+            }
+            super::time("open_maps", || ());
+            2
+        }
+        std::thread::spawn(|| {
+            super::record_here(true);
+            assert_eq!(step(true), 1);
+            assert_eq!(step(false), 2);
+            super::time("use_item", || ());
+            let raw = super::take_raw();
+            let seen: Vec<_> = raw.iter().map(|s| (s.name, s.depth)).collect();
+            assert_eq!(
+                seen,
+                [
+                    ("open_loot", 1),
+                    ("use_block", 0),
+                    ("open_loot", 1),
+                    ("open_maps", 1),
+                    ("use_block", 0),
+                    ("use_item", 0)
+                ]
+            );
+        })
+        .join()
+        .unwrap();
+    }
+
     /// A slow frame is put down to the slowest outer span, then to the span inside it that took
     /// at least half of it; repeated spans (several ticks a frame) add up.
     #[test]

crates/horizon-client/src/game.rs

@@ -5720,13 +5720,15 @@ impl Game<'_> {
             // ShulkerBoxBlock.canOpen: the lid has no room; nothing opens.
         } else if let Some(entity) = BlockEntity::for_block(&name) {
             self.world.block_entities.get_or_insert_with(key, || entity);
-            self.unpack_opened_loot(key, &name);
+            horizon_client::spans::time("open_loot", || self.unpack_opened_loot(key, &name));
             let centre = [
                 f64::from(key.0) + 0.5,
                 f64::from(key.1) + 0.5,
                 f64::from(key.2) + 0.5,
             ];
-            self.resolve_container_maps(key, centre);
+            horizon_client::spans::time("open_maps", || {
+                self.resolve_container_maps(key, centre);
+            });
             self.container = Some(key);
             let first = self.container_opener_event(key, true);
             self.post_block_game_event(
@@ -6374,9 +6376,13 @@ impl Game<'_> {
         let mut anchor_passed = false;
         // Native mods, where ServerPlayerGameMode.useItemOn starts: a
         // canceled click does nothing (a guest's block is sent back).
+        // A slow tick's line names the phase of a use that took it (#8669): the mods' hook,
+        // the block's own use, then the item's.
         if let Some(hit) = block_hit {
             let mut canceled = false;
-            self.native_right_click_block(hit.pos, hit.face, &mut canceled);
+            horizon_client::spans::time("use_mods", || {
+                self.native_right_click_block(hit.pos, hit.face, &mut canceled);
+            });
             if canceled {
                 // Nothing was used, so nothing consumed the action.
                 self.use_failed = true;
@@ -6384,6 +6390,7 @@ impl Game<'_> {
             }
         }
         // MultiPlayerGameMode.useItemOn: blocks react first.
+        let block_phase = horizon_client::spans::guard("use_block");
         if !(sneaking && hands_full)
             && let Some(hit) = block_hit
         {
@@ -6566,6 +6573,8 @@ impl Game<'_> {
                 }
             }
         }
+        drop(block_phase);
+        let _item_phase = horizon_client::spans::guard("use_item");
         let Some(stack) = self.held().cloned() else {
             return Ok(false);
         };

crates/horizon-client/src/game/lan.rs

@@ -1840,7 +1840,8 @@ impl Game<'_> {
         for event in events {
             if timed {
                 note_event(&host, previous.take());
-                previous = Some((Instant::now(), event_kind(&event)));
+                let detail = self.event_detail(&host, &event);
+                previous = Some((Instant::now(), event_kind(&event), detail));
             }
             match event {
                 event @ (HostEvent::Chat { .. } | HostEvent::Warn(_) | HostEvent::Error(_)) => {
@@ -3060,22 +3061,73 @@ fn event_kind(event: &HostEvent) -> EventKind {
     }
 }
 
-/// Notes how long the event that started at `started` took (`spans::note`).
-fn note_event(host: &LanHost, started: Option<(Instant, EventKind)>) {
-    let Some((started, (kind, conn, name))) = started else {
+/// Notes how long the event that started at `started` took (`spans::note`), with what it
+/// was about when [`Game::event_detail`] said.
+fn note_event(host: &LanHost, started: Option<(Instant, EventKind, Option<UseDetail>)>) {
+    let Some((started, (kind, conn, name), detail)) = started else {
         return;
     };
     horizon_client::spans::note(started.elapsed().as_secs_f32() * 1000.0, || {
         let who = name
             .or_else(|| conn.and_then(|c| host.hub.guest(c)).map(|g| g.name.clone()))
             .or_else(|| conn.map(|c| format!("connection {c}")));
-        match who {
+        let line = match who {
             Some(who) => format!("{kind} of {who}"),
             None => kind.to_owned(),
+        };
+        match detail {
+            Some(d) => format!("{line} ({d})"),
+            None => line,
         }
     });
 }
 
+/// What a use packet was about, taken before it ran (the use may consume the item or change
+/// the block): the item in the hand it names and, on a block, that block, where, and the face.
+struct UseDetail {
+    item: Option<Arc<str>>,
+    block: Option<(Arc<str>, [i32; 3], i32)>,
+}
+
+impl std::fmt::Display for UseDetail {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(self.item.as_deref().unwrap_or("empty hand"))?;
+        if let Some((block, [x, y, z], face)) = &self.block {
+            let face = ["down", "up", "north", "south", "west", "east"]
+                .get(*face as usize)
+                .copied()
+                .unwrap_or("?");
+            write!(f, " on {block} at {x} {y} {z} {face}")?;
+        }
+        Ok(())
+    }
+}
+
+impl Game<'_> {
+    /// For a slow tick's note (#8669): the item a `UseItemOn` or `UseItem` uses and the block
+    /// it aims at. None for other events.
+    fn event_detail(&self, host: &LanHost, event: &HostEvent) -> Option<UseDetail> {
+        let HostEvent::Packet(conn, packet) = event else {
+            return None;
+        };
+        let (hand, block) = match &**packet {
+            play::Serverbound::UseItemOn(p) => (p.hand, Some((p.pos, p.face))),
+            play::Serverbound::UseItem(p) => (p.hand, None),
+            _ => return None,
+        };
+        let guest = host.hub.guest(*conn)?;
+        let held = if hand == 1 {
+            guest.offhand.as_ref()
+        } else {
+            guest.inventory.get(guest.selected).and_then(Option::as_ref)
+        };
+        Some(UseDetail {
+            item: held.map(|s| Arc::clone(&s.item)),
+            block: block.map(|(pos, face)| (self.block_name(pos), pos, face)),
+        })
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::{OFFHAND_SLOT, equipment_slot_name, living_flags, mob_freeze_meta};

crates/horizon-client/src/game/lan_actions.rs

@@ -2096,7 +2096,9 @@ impl Game<'_> {
                             let stack = g
                                 .with_hand(hand, |g| g.held().cloned())
                                 .unwrap_or_else(|| ItemStack::new("minecraft:air", 0));
-                            g.trigger_any_block_use(p.pos, &stack);
+                            horizon_client::spans::time("use_advancements", || {
+                                g.trigger_any_block_use(p.pos, &stack);
+                            });
                         }
                         g.forced_hit = None;
                         g.guest_use = None;

crates/horizon-client/src/old_world.rs

@@ -26,6 +26,9 @@ pub struct SavedOldWorld {
     data: Arc<GeneratorData>,
     /// The old chunks of each region file (`SaveDir::old_chunk_bits`).
     regions: Mutex<HashMap<(i32, i32), Arc<OnceLock<[u64; 16]>>>>,
+    /// Chunks asked about one at a time in regions not scanned ([`OldWorld::is_old_chunk`]):
+    /// per region, the chunks read and the old ones among them.
+    single: Mutex<HashMap<(i32, i32), ([u64; 16], [u64; 16])>>,
     /// Each old chunk's blending data, its border columns calculated.
     blending: Mutex<HashMap<(i32, i32), Option<Arc<BlendingData>>>>,
     kept_biomes: Mutex<HashMap<(i32, i32), Option<Arc<BiomeChunk<BiomeId>>>>>,
@@ -43,6 +46,7 @@ impl SavedOldWorld {
             biomes,
             data,
             regions: Default::default(),
+            single: Default::default(),
             blending: Default::default(),
             kept_biomes: Default::default(),
         }
@@ -152,6 +156,28 @@ impl OldWorld for SavedOldWorld {
         }
         false
     }
+    fn is_old_chunk(&self, x: i32, z: i32) -> bool {
+        let key = (x >> 5, z >> 5);
+        let index = ((x & 31) + (z & 31) * 32) as usize;
+        let (word, bit) = (index / 64, 1u64 << (index % 64));
+        let scanned = (self.regions.lock().unwrap().get(&key)).and_then(|cell| cell.get().copied());
+        if let Some(bits) = scanned {
+            return bits[word] & bit != 0;
+        }
+        if let Some((read, old)) = self.single.lock().unwrap().get(&key)
+            && read[word] & bit != 0
+        {
+            return old[word] & bit != 0;
+        }
+        let old = self.save.is_old_chunk(x, z);
+        let mut single = self.single.lock().unwrap();
+        let (read, olds) = single.entry(key).or_default();
+        read[word] |= bit;
+        if old {
+            olds[word] |= bit;
+        }
+        old
+    }
     fn saved_starts(&self, x: i32, z: i32) -> Vec<horizon_world::worldgen::blending::SavedStart> {
         self.save
             .raw_chunk(x, z)

crates/horizon-client/src/save.rs

@@ -2562,17 +2562,19 @@ impl SaveDir {
             let Some(root) = self.read_record_raw("region", pos.x, pos.z) else {
                 continue;
             };
-            let old = match root.tag.get("DataVersion") {
-                Some(Tag::Int(version)) => *version < 4997,
-                _ => true,
-            } || root.tag.get("blending_data").is_some();
-            if old {
+            if record_is_old(&root) {
                 let index = ((pos.x & 31) + (pos.z & 31) * 32) as usize;
                 bits[index / 64] |= 1 << (index % 64);
             }
         }
         bits
     }
+    /// Whether the saved chunk at `x, z` is old ([`Self::old_chunk_bits`]'s test), from its
+    /// record alone: false for a chunk not saved.
+    pub fn is_old_chunk(&self, x: i32, z: i32) -> bool {
+        self.read_record_raw("region", x, z)
+            .is_some_and(|root| record_is_old(&root))
+    }
     /// A saved chunk brought to the current version, for the readers of its
     /// fields that `load` does not decode (`blending_data`).
     pub fn raw_chunk(&self, x: i32, z: i32) -> Option<NamedTag> {
@@ -5064,6 +5066,15 @@ impl<G: ChunkGenerator> ChunkGenerator for Persistent<G> {
     }
 }
 
+/// `IOWorker.createOldDataForRegion`'s test of one record: saved before DataVersion 4997, or
+/// with `blending_data`.
+fn record_is_old(root: &NamedTag) -> bool {
+    let before_blending = match root.tag.get("DataVersion") {
+        Some(Tag::Int(version)) => *version < 4997,
+        _ => true,
+    };
+    before_blending || root.tag.get("blending_data").is_some()
+}
 #[cfg(test)]
 mod tests {
     use crate::mobs::spawner::SpawnDataExt;
@@ -7440,6 +7451,30 @@ mod tests {
         let upgraded = nether.raw_chunk(0, 0).expect("the chunk reads");
         assert!(upgraded.tag.get("blending_data").is_none());
         assert_ne!(nether.old_chunk_bits(0, 0)[0] & 1, 0);
+        // One chunk's record tells the same, without the region's other chunks; a chunk this
+        // version saved, and one not saved, are not old.
+        let current = Record::Tag(Arc::new(NamedTag {
+            name: "".into(),
+            tag: Tag::compound([
+                ("DataVersion", Tag::Int(DATA_VERSION)),
+                ("xPos", Tag::Int(1)),
+                ("zPos", Tag::Int(0)),
+                ("yPos", Tag::Int(0)),
+                ("Status", string("minecraft:full")),
+                ("sections", Tag::List(Vec::new())),
+            ]),
+        }));
+        super::write_region_records(
+            &nether.root,
+            &nether.regions,
+            "region",
+            &[((1, 0), &current)],
+        )
+        .unwrap();
+        assert!(nether.is_old_chunk(0, 0));
+        assert!(!nether.is_old_chunk(1, 0));
+        assert!(!nether.is_old_chunk(2, 0));
+        assert_eq!(nether.old_chunk_bits(0, 0)[0] & 0b110, 0);
         // A POI record of before 1.14.4 (no DataVersion, read as 1945): its
         // sections at the root, which 1946 moved under `Sections`.
         let overworld = SaveDir::new(

crates/horizon-world/src/worldgen/blending.rs

@@ -404,6 +404,13 @@ pub trait OldWorld: Send + Sync {
     /// `WorldGenRegion.isOldChunkAround`: a chunk within `range` of `center`
     /// (a square) is old, as far as the storage can tell without reading it.
     fn is_old_chunk_around(&self, center: ChunkPos, range: i32) -> bool;
+    /// Whether the chunk at `x, z` is old, as `StructureCheck.tryLoadFromStorage` finds out:
+    /// from that chunk's record alone. A structure search asks this of one chunk per spacing
+    /// cell across thousands of cells; scanning each region it touched, as the blender does,
+    /// read every chunk an explored world saved there (#8669).
+    fn is_old_chunk(&self, x: i32, z: i32) -> bool {
+        self.is_old_chunk_around(ChunkPos::new(x, z), 0)
+    }
     /// The blending data of the old chunk at `x, z`, its border columns
     /// calculated, or None for a chunk that is not old.
     fn blending(&self, x: i32, z: i32) -> Option<Arc<BlendingData>>;

crates/horizon-world/src/worldgen/generator.rs

@@ -2014,7 +2014,7 @@ where
         self.pipeline
             .old_world
             .get()
-            .is_some_and(|world| world.is_old_chunk_around(chunk, 0))
+            .is_some_and(|world| world.is_old_chunk(chunk.x, chunk.z))
     }
     fn saved_starts(&self, chunk: ChunkPos) -> Vec<super::blending::SavedStart> {
         self.pipeline
@@ -2455,6 +2455,16 @@ trait SeededPipeline: Send + Sync {
     ) -> Result<(), String>;
     fn forget_structure_starts(&self);
     fn ring_positions(&self, worker: &mut WorkerState, set: &str) -> Option<Vec<ChunkPos>>;
+    /// The structures whose random-spread set may start in `chunk`, each with whether a
+    /// search's start check lets it start there.
+    #[cfg(test)]
+    fn may_start_answers(
+        &self,
+        _worker: &mut WorkerState,
+        _chunk: ChunkPos,
+    ) -> Vec<(String, bool)> {
+        Vec::new()
+    }
     fn spawn_position(&self) -> Result<[i32; 2], GeneratorError>;
     fn region_random(&self, center: ChunkPos) -> Box<dyn super::structure::legacy::PieceRandom>;
     fn locate(
@@ -2624,6 +2634,20 @@ where
             structures.forget_starts();
         }
     }
+    #[cfg(test)]
+    fn may_start_answers(&self, worker: &mut WorkerState, chunk: ChunkPos) -> Vec<(String, bool)> {
+        match &self.structures {
+            Some(structures) => structures.may_start_answers(
+                &mut StructureQueries {
+                    pipeline: self,
+                    worker,
+                    valid: BTreeSet::new(),
+                },
+                chunk,
+            ),
+            None => Vec::new(),
+        }
+    }
     fn ring_positions(&self, worker: &mut WorkerState, set: &str) -> Option<Vec<ChunkPos>> {
         self.structures.as_ref()?.ring_positions(
             &mut StructureQueries {
@@ -3729,6 +3753,10 @@ impl GeneratorWorker {
     pub fn forget_structure_starts(&mut self) {
         self.pipeline.forget_structure_starts();
     }
+    #[cfg(test)]
+    fn may_start_answers(&mut self, chunk: ChunkPos) -> Vec<(String, bool)> {
+        self.pipeline.may_start_answers(&mut self.state, chunk)
+    }
     pub fn stage_timings(&self) -> StageTimings {
         self.state.timings
     }
@@ -4282,6 +4310,44 @@ mod tests {
         assert_eq!(before, after);
     }
 
+    /// A map's structure search skips a chunk's starts when no wanted structure's start
+    /// piece and biome fit there (#8669). Over the candidate chunks of every random-spread
+    /// set around the camp TiesB2007's map went looking from, a skipped structure is never
+    /// one the chunk's full starts hold, and some are skipped.
+    #[test]
+    fn may_start_never_skips_a_start_the_chunk_holds() {
+        let Some(data) = prepared_data() else {
+            return;
+        };
+        let generator = OverworldGenerator::new(data, -3292718948513116954).expect("generator");
+        let mut worker = generator.worker();
+        let (mut held, mut skipped) = (0, 0);
+        for x in 212..332 {
+            for z in -481..-361 {
+                let chunk = ChunkPos::new(x, z);
+                let checks = worker.may_start_answers(chunk);
+                if checks.is_empty() {
+                    continue;
+                }
+                let starts: Vec<String> = (worker.structure_starts(chunk).iter())
+                    .map(|s| s.name.clone())
+                    .collect();
+                for (name, may_start) in checks {
+                    if starts.contains(&name) {
+                        assert!(
+                            may_start,
+                            "{name} starts in {chunk:?}, but its check skips it"
+                        );
+                        held += 1;
+                    } else if !may_start {
+                        skipped += 1;
+                    }
+                }
+            }
+        }
+        assert!(held > 0 && skipped > 0, "held {held}, skipped {skipped}");
+    }
+
     /// Generators of one seed and dimension share their ring positions;
     /// another seed or generator gets its own, and old entries go.
     #[test]

crates/horizon-world/src/worldgen/generator_structures.rs

@@ -8,7 +8,7 @@ use super::{
         jigsaw::PoolPiece,
         start::{
             Piece, Rings, StartContext, StructureRegistry, StructureStart, beardifier,
-            create_starts, try_generate,
+            create_starts, may_start, try_generate,
         },
         stronghold::RingSearch,
     },
@@ -379,6 +379,39 @@ impl StructureState {
             .any(|(_, search)| search.may_land_near(chunk.x, chunk.z, self.zone_reach));
         if near { self.rings(context) } else { &NONE }
     }
+    /// The structures of the random-spread sets whose start chunk is `chunk`, each with
+    /// [`may_start`]'s answer, for tests.
+    #[cfg(test)]
+    pub(super) fn may_start_answers(
+        &self,
+        context: &mut impl super::structure::start::StartContext,
+        chunk: ChunkPos,
+    ) -> Vec<(String, bool)> {
+        let seed = context.seed();
+        let rings = self.rings_for(context, chunk).clone();
+        let mut checks = Vec::new();
+        for &si in &self.possible_sets {
+            let set = &self.registry.sets[si];
+            if set.spacing().is_none()
+                || StructureRegistry::potential_chunk(set, seed, chunk.x, chunk.z) != Some(chunk)
+                || !(self.registry).is_structure_chunk(set, seed, chunk.x, chunk.z, &rings)
+            {
+                continue;
+            }
+            for &(s, _) in &set.structures {
+                let may = may_start(
+                    &self.registry,
+                    &self.resources,
+                    &self.bindings,
+                    context,
+                    s,
+                    chunk,
+                );
+                checks.push((self.registry.structures[s].name.clone(), may));
+            }
+        }
+        checks
+    }
     /// `StructureRegistry::needs_dimension_origin`: whether any set is placed
     /// by the dimension's origin chunk alone.
     pub(super) fn needs_dimension_origin(&self) -> bool {
@@ -602,9 +635,9 @@ impl StructureState {
     /// its locate position (the start chunk's corner at y 0). `skip` says a
     /// start (structure name, its chunk) can't be referenced again
     /// (`StructureStart.canBeReferenced`): the search goes on past it.
-    pub(super) fn locate(
+    pub(super) fn locate<C: super::structure::start::StartContext>(
         &self,
-        context: &mut impl super::structure::start::StartContext,
+        context: &mut C,
         names: &[String],
         from: [i32; 3],
         radius: i32,
@@ -618,7 +651,30 @@ impl StructureState {
             dx * dx + dz * dz
         };
         let mut best: Option<(f64, String, [i32; 3])> = None;
-        let found_at = |this: &Self, context: &mut _, chunk: ChunkPos| -> Option<String> {
+        let wanted_structures: Vec<usize> = (0..self.registry.structures.len())
+            .filter(|&s| wanted(s))
+            .collect();
+        let found_at = |this: &Self, context: &mut C, chunk: ChunkPos| -> Option<String> {
+            // StructureCheck.checkStart: a chunk with no starts built or saved yet is first
+            // asked whether a wanted structure could start there (canCreateStructure, a
+            // jigsaw's start piece and its biome), and only then are all its starts built,
+            // layouts included. A map's search for a camp of a rarer biome built thousands of
+            // chunks' starts on the server thread, seconds of it (#8669).
+            if this.starts.get((chunk.x, chunk.z)).is_none()
+                && !context.is_old_chunk(chunk)
+                && !wanted_structures.iter().any(|&s| {
+                    may_start(
+                        &this.registry,
+                        &this.resources,
+                        &this.bindings,
+                        context,
+                        s,
+                        chunk,
+                    )
+                })
+            {
+                return None;
+            }
             this.starts_at(context, chunk)
                 .iter()
                 .find(|s| wanted(s.structure) && !skip(&s.name, chunk))

crates/horizon-world/src/worldgen/structure/jigsaw.rs

@@ -1075,6 +1075,37 @@ pub fn assemble_drawing(
     random: &mut impl RandomSource,
     position: [i32; 3],
 ) -> Option<([i32; 3], Vec<PoolPiece>)> {
+    let start = place_start(
+        draws, config, context, pools, resources, bindings, random, position,
+    )?;
+    Some(lay_out_children(
+        draws, start, config, context, pools, resources, bindings, random,
+    ))
+}
+
+/// A jigsaw's start piece in place ([`place_start`]), its children not laid out yet: vanilla's
+/// `Structure.GenerationStub`, whose position the structure's biome is checked at before
+/// `JigsawPlacement.addPieces` lays the rest out.
+pub struct PlacedStart {
+    /// The start position (`[centre x, start y, centre z]`).
+    pub position: [i32; 3],
+    center: PoolPiece,
+}
+
+/// The first half of [`assemble_drawing`]: the start piece drawn, rotated, projected to the
+/// heightmap and checked against the height limits. None when the start is invalid. The
+/// draws it makes come first in `random`, as in `assemble_drawing`.
+#[allow(clippy::too_many_arguments)]
+pub fn place_start(
+    draws: Draws,
+    config: &JigsawConfig<'_>,
+    context: &mut impl JigsawContext,
+    pools: &Pools,
+    resources: &StructureResources,
+    bindings: &FeatureBlockBindings<'_>,
+    random: &mut impl RandomSource,
+    position: [i32; 3],
+) -> Option<PlacedStart> {
     let alias = |name: &str| {
         config
             .aliases
@@ -1144,6 +1175,29 @@ pub fn assemble_drawing(
         return None;
     }
     let cy = bottom + local[1];
+    Some(PlacedStart {
+        position: [cx, cy, cz],
+        center,
+    })
+}
+
+/// The second half of [`assemble_drawing`]: the children of a [`place_start`]ed start laid out
+/// to `config.max_depth`, with the same `random`. Returns the start position and every piece.
+#[allow(clippy::too_many_arguments)]
+pub fn lay_out_children(
+    draws: Draws,
+    start: PlacedStart,
+    config: &JigsawConfig<'_>,
+    context: &mut impl JigsawContext,
+    pools: &Pools,
+    resources: &StructureResources,
+    bindings: &FeatureBlockBindings<'_>,
+    random: &mut impl RandomSource,
+) -> ([i32; 3], Vec<PoolPiece>) {
+    let PlacedStart {
+        position: [cx, cy, cz],
+        center,
+    } = start;
     let mut pieces = vec![center];
     if config.max_depth > 0 {
         let [h, v] = config.max_distance;
@@ -1164,7 +1218,7 @@ pub fn assemble_drawing(
             container,
         );
     }
-    Some(([cx, cy, cz], pieces))
+    ([cx, cy, cz], pieces)
 }
 
 /// The inner JigsawPlacement.addPieces: lays out the children of

crates/horizon-world/src/worldgen/structure/start.rs

@@ -3,7 +3,8 @@
 use super::{
     BoundingBox, StructureResources,
     jigsaw::{
-        JigsawConfig, JigsawContext, PlaceFeature, PoolPiece, Pools, Projection, assemble_drawing,
+        JigsawConfig, JigsawContext, PlaceFeature, PoolPiece, Pools, Projection, lay_out_children,
+        place_start,
     },
     legacy::{LegacyPiece, PieceContext},
 };
@@ -1018,7 +1019,53 @@ pub(crate) fn try_generate(
     chunk: ChunkPos,
     any_biome: bool,
 ) -> Option<StructureStart> {
+    match generate(
+        registry, resources, bindings, context, structure, chunk, any_biome, false,
+    )? {
+        Generated::Start(start) => Some(start),
+        Generated::Stub => None,
+    }
+}
+/// `StructureCheck.canCreateStructure`: whether `structure` may start in `chunk`, by
+/// `Structure.findValidGenerationPoint` alone. A jigsaw's start piece is placed and its biome
+/// checked; its children are not laid out. Other structures answer true: their starts are
+/// cheap to build. False only where [`try_generate`] can't start it, so a search may skip
+/// the chunk's full starts then (#8669).
+pub(crate) fn may_start(
+    registry: &StructureRegistry,
+    resources: &StructureResources,
+    bindings: &FeatureBlockBindings<'_>,
+    context: &mut impl StartContext,
+    structure: usize,
+    chunk: ChunkPos,
+) -> bool {
+    generate(
+        registry, resources, bindings, context, structure, chunk, false, true,
+    )
+    .is_some()
+}
+/// What [`generate`] made: the start, or with `stub_only` only that it may start.
+enum Generated {
+    Start(StructureStart),
+    Stub,
+}
+#[allow(clippy::too_many_arguments)]
+fn generate(
+    registry: &StructureRegistry,
+    resources: &StructureResources,
+    bindings: &FeatureBlockBindings<'_>,
+    context: &mut impl StartContext,
+    structure: usize,
+    chunk: ChunkPos,
+    any_biome: bool,
+    stub_only: bool,
+) -> Option<Generated> {
     let def = &registry.structures[structure];
+    let vanilla_jigsaw =
+        matches!(&def.kind, StructureKind::Jigsaw(jigsaw) if jigsaw.piece_limited.is_none());
+    if stub_only && !vanilla_jigsaw {
+        return Some(Generated::Stub);
+    }
     context.set_valid_biomes((!any_biome).then_some(&def.biomes));
     let seed = context.seed();
     let mut random = WorldgenRandom::new(LegacyRandom::new(0));
@@ -1061,16 +1108,38 @@ pub(crate) fn try_generate(
                     chunk,
                     start,
                 )?,
-                None => assemble_drawing(
-                    jigsaw.draws,
-                    &config,
-                    context,
-                    &registry.pools,
-                    resources,
-                    bindings,
-                    &mut random,
-                    start,
-                )?,
+                None => {
+                    let placed = place_start(
+                        jigsaw.draws,
+                        &config,
+                        context,
+                        &registry.pools,
+                        resources,
+                        bindings,
+                        &mut random,
+                        start,
+                    )?;
+                    // Structure.findValidGenerationPoint checks the stub's biome before its
+                    // GenerationStub lays the pieces out: a start in another biome costs its
+                    // start piece, not its whole layout (an ancient city's, a camp's).
+                    let quart = placed.position.map(|v| v >> 2);
+                    if !any_biome && !def.biomes.contains(&context.noise_biome(quart)) {
+                        return None;
+                    }
+                    if stub_only {
+                        return Some(Generated::Stub);
+                    }
+                    lay_out_children(
+                        jigsaw.draws,
+                        placed,
+                        &config,
+                        context,
+                        &registry.pools,
+                        resources,
+                        bindings,
+                        &mut random,
+                    )
+                }
             };
             // Vanilla's JigsawPlacement keeps no piece at size 0; the
             // piece-limited assembler keeps the start piece.
@@ -1331,7 +1400,9 @@ pub(crate) fn try_generate(
     if (!any_biome && !def.biomes.contains(&context.noise_biome(quart))) || pieces.is_empty() {
         return None;
     }
-    Some(StructureStart::new(structure, def, chunk, pieces))
+    Some(Generated::Start(StructureStart::new(
+        structure, def, chunk, pieces,
+    )))
 }
 impl StructureStart {
     /// A start of `def` (registry index `structure`) from its pieces, at