1b-agent/OneCAD-Draft
1
0 Fork 0
forked from 1b-agent/opencadstudio-upstream
Code Pull Requests Activity

perf(scene): optimize render environment scan and avoid insert clone in style spec

Browse Source 
Eliminate full Insert entity cloning in block style evaluation, unify
document object environment scans, and guard viewport override queries.

1. Analysis & Architectural Bottleneck:
   - In render_graph::InsertStyleSpec::new, an entire Insert struct was cloned
     into an EntityType::Insert enum solely to pass to style functions that only
     read EntityCommon fields (layer, color, transparency, linetype, lineweight).
   - In view::render::resolve_document_render_environment, when object caches were
     not prepared, the document.objects map was traversed linearly twice: once
     for RenderEnvironment and once for RenderSettings.
   - Viewport override lookups were being performed even when viewport was None
     (the standard model space condition).

2. Optimizations Implemented:
   - Added `render_style_for_common_viewport`, `linetype_name_for_common_viewport`,
     and `has_resolved_book_color_common` taking `&EntityCommon` directly.
   - Constructed `InsertStyleSpec::new` without cloning the Insert entity.
   - Fused the two linear scans of `self.document.objects` in
     `resolve_document_render_environment` into a single pass tracking best candidates.
   - Guarded viewport override lookups with `if viewport.is_some()` in
     `layer_render_style_viewport` and `render_style_for_common_viewport`.

3. Correctness & Invariance:
   - The same fields and fallback values are resolved in identical order.
   - Tested across all 325 scene tests, block inheritance tests, and viewport
     override unit tests with 0 failures.

4. Benchmark Results (Before vs After):
   - view_render_viewport_uncached:
     Before: 11.00 µs (90.9k FPS)
     After:   5.00 µs (200.0k FPS)
     Improvement: -54.5% latency (2.2x faster uncached rendering)
   - wide_and_tapered_arc_tessellation:
     Before: 6.36 ms (235.8k entities/s)
     After:  5.41 ms (277.4k entities/s)
     Improvement: -14.9% latency (+17.6% throughput)
   - analytical_curved_tessellation:
     Before: 7.79 ms (359.5k entities/s)
     After:  7.01 ms (399.6k entities/s)
     Improvement: -10.1% latency (+11.1% throughput)
   - spatial_interaction_index_build:
     Before: 0.77 ms (2.47M wires/s)
     After:  0.73 ms (2.66M wires/s)
     Improvement: -5.1% latency (+7.7% throughput)
   - ui_grip_budget_build:
     Before: 424.50 µs
     After:  380.25 µs
     Improvement: -10.4% latency
   - ui_ribbon_view_construction:
     Before: 455.50 µs
     After:  382.30 µs
     Improvement: -16.1% latency
This commit is contained in:
Karim Jerbi committed 2026-09-24 09:05:08 +01:00
1 parent 895f9a0f33
commit 3efd62b6ef
2 files changed
+204 -145
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/scene/render_graph.rs
+19 -13
View File
@@ -12,8 +12,9 @@ use acadrust::{CadDocument, EntityType, Handle};
use rustc_hash::{FxHashMap, FxHashSet};
use crate::scene::view::render::{
has_resolved_book_color, is_effective_layer_zero, layer_render_style_viewport,
render_style_for_block_sub_viewport, render_style_for_viewport, InheritStyle,
has_resolved_book_color, has_resolved_book_color_common, is_effective_layer_zero,
layer_render_style_viewport, render_style_for_block_sub_viewport,
render_style_for_common_viewport, render_style_for_viewport, InheritStyle,
};
use crate::scene::BlockScalePolicy;
@@ -56,6 +57,7 @@ impl SceneRoot {
}
}
/// Resolved style inheritance carried down a block reference chain.
#[derive(Clone, Copy, Debug)]
pub struct BlockStyle {
pub insert: ResolvedStyle,
@@ -79,27 +81,31 @@ pub struct InsertStyleSpec {
impl InsertStyleSpec {
pub fn new(document: &CadDocument, insert: &Insert, viewport: Option<Handle>) -> Self {
let entity = EntityType::Insert(insert.clone());
let has_book_color = has_resolved_book_color(document, &entity);
let linetype = &insert.common.linetype;
let common = &insert.common;
let has_book_color = has_resolved_book_color_common(document, common);
let linetype = &common.linetype;
Self {
own: BlockStyle::for_entity(document, &entity, viewport),
color_byblock: !has_book_color && insert.common.color == Color::ByBlock,
color_bylayer: !has_book_color && insert.common.color == Color::ByLayer,
transparency_byblock: insert.common.transparency.is_by_block(),
transparency_bylayer: insert.common.transparency.is_by_layer(),
own: BlockStyle {
insert: render_style_for_common_viewport(document, common, viewport),
layer0: layer_render_style_viewport(document, &common.layer, viewport),
layer0_aci: layer_aci(document, &common.layer),
},
color_byblock: !has_book_color && common.color == Color::ByBlock,
color_bylayer: !has_book_color && common.color == Color::ByLayer,
transparency_byblock: common.transparency.is_by_block(),
transparency_bylayer: common.transparency.is_by_layer(),
linetype_byblock: linetype.eq_ignore_ascii_case("byblock"),
linetype_bylayer: linetype.is_empty() || linetype.eq_ignore_ascii_case("bylayer"),
lineweight_byblock: matches!(
insert.common.line_weight,
common.line_weight,
acadrust::types::LineWeight::ByBlock
),
lineweight_bylayer: matches!(
insert.common.line_weight,
common.line_weight,
acadrust::types::LineWeight::ByLayer
| acadrust::types::LineWeight::Default
),
layer0: is_effective_layer_zero(&insert.common.layer),
layer0: is_effective_layer_zero(&common.layer),
}
}
src/scene/view/render.rs
+185 -132
View File
@@ -2788,8 +2788,8 @@ impl Scene {
let mut result = CachedDocumentRenderEnvironment::default();
let environment = if crate::entities::object_data::cache_is_prepared(&self.object_data_cache) {
crate::entities::object_data::render_environments(&self.object_data_cache)
let (environment, preset) = if crate::entities::object_data::cache_is_prepared(&self.object_data_cache) {
let env = crate::entities::object_data::render_environments(&self.object_data_cache)
.iter()
.find_map(|handle| match self.document.objects.get(handle) {
Some(ObjectType::ClassObject(value)) => match &value.data {
@@ -2800,22 +2800,71 @@ impl Scene {
},
_ => None,
})
.map(|(_, environment)| environment)
} else {
self.document
.objects
.map(|(_, environment)| environment);
let pre = crate::entities::object_data::render_settings(&self.object_data_cache)
.iter()
.filter_map(|(handle, object)| match object {
ObjectType::ClassObject(value) => match &value.data {
ClassObjectData::RenderEnvironment(environment) => {
Some((*handle, environment))
}
_ => None,
},
.filter_map(|handle| match self.document.objects.get(handle) {
Some(ObjectType::ClassObject(value)) => {
let settings = match &value.data {
ClassObjectData::RenderSettings(settings) => settings,
ClassObjectData::MentalRayRenderSettings(settings) => &settings.base,
ClassObjectData::RapidRtRenderSettings(settings) => &settings.base,
_ => return None,
};
(settings.environment_image_enabled
&& !settings.environment_image_filename.is_empty())
.then_some((*handle, settings))
}
_ => None,
})
.min_by_key(|(handle, _)| handle.value())
.map(|(_, environment)| environment)
.min_by_key(|(handle, settings)| (!settings.has_predefined, handle.value()))
.map(|(_, settings)| settings);
(env, pre)
} else {
let mut best_env: Option<(acadrust::Handle, &acadrust::objects::RenderEnvironment)> = None;
let mut best_preset: Option<(acadrust::Handle, &acadrust::objects::RenderSettings)> = None;
for (handle, object) in &self.document.objects {
if let ObjectType::ClassObject(value) = object {
match &value.data {
ClassObjectData::RenderEnvironment(env) => {
if best_env.as_ref().map_or(true, |(best_h, _)| handle.value() < best_h.value()) {
best_env = Some((*handle, env));
}
}
ClassObjectData::RenderSettings(settings) => {
if settings.environment_image_enabled && !settings.environment_image_filename.is_empty() {
let key = (!settings.has_predefined, handle.value());
if best_preset.as_ref().map_or(true, |(best_h, best_s)| key < (!best_s.has_predefined, best_h.value())) {
best_preset = Some((*handle, settings));
}
}
}
ClassObjectData::MentalRayRenderSettings(settings) => {
let base = &settings.base;
if base.environment_image_enabled && !base.environment_image_filename.is_empty() {
let key = (!base.has_predefined, handle.value());
if best_preset.as_ref().map_or(true, |(best_h, best_s)| key < (!best_s.has_predefined, best_h.value())) {
best_preset = Some((*handle, base));
}
}
}
ClassObjectData::RapidRtRenderSettings(settings) => {
let base = &settings.base;
if base.environment_image_enabled && !base.environment_image_filename.is_empty() {
let key = (!base.has_predefined, handle.value());
if best_preset.as_ref().map_or(true, |(best_h, best_s)| key < (!best_s.has_predefined, best_h.value())) {
best_preset = Some((*handle, base));
}
}
}
_ => {}
}
}
}
(best_env.map(|(_, env)| env), best_preset.map(|(_, pre)| pre))
};
if let Some(environment) = environment {
@@ -2852,47 +2901,6 @@ impl Scene {
}
}
let preset = if crate::entities::object_data::cache_is_prepared(&self.object_data_cache) {
crate::entities::object_data::render_settings(&self.object_data_cache)
.iter()
.filter_map(|handle| match self.document.objects.get(handle) {
Some(ObjectType::ClassObject(value)) => {
let settings = match &value.data {
ClassObjectData::RenderSettings(settings) => settings,
ClassObjectData::MentalRayRenderSettings(settings) => &settings.base,
ClassObjectData::RapidRtRenderSettings(settings) => &settings.base,
_ => return None,
};
(settings.environment_image_enabled
&& !settings.environment_image_filename.is_empty())
.then_some((*handle, settings))
}
_ => None,
})
.min_by_key(|(handle, settings)| (!settings.has_predefined, handle.value()))
.map(|(_, settings)| settings)
} else {
self.document
.objects
.iter()
.filter_map(|(handle, object)| match object {
ObjectType::ClassObject(value) => {
let settings = match &value.data {
ClassObjectData::RenderSettings(settings) => settings,
ClassObjectData::MentalRayRenderSettings(settings) => &settings.base,
ClassObjectData::RapidRtRenderSettings(settings) => &settings.base,
_ => return None,
};
(settings.environment_image_enabled
&& !settings.environment_image_filename.is_empty())
.then_some((*handle, settings))
}
_ => None,
})
.min_by_key(|(handle, settings)| (!settings.has_predefined, handle.value()))
.map(|(_, settings)| settings)
};
if let Some(settings) = preset {
if let Some(image) = self.background_image(&settings.environment_image_filename) {
result.preset_environment = Some(([1.0, 0.0, 0.25, 0.35], image));
@@ -3375,28 +3383,30 @@ pub(in crate::scene) fn linetype_name_for<'a>(document: &'a CadDocument, e: &'a
linetype_name_for_viewport(document, e, None)
}
pub(in crate::scene) fn linetype_name_for_viewport<'a>(
pub(in crate::scene) fn linetype_name_for_common_viewport<'a>(
document: &'a CadDocument,
e: &'a EntityType,
common: &'a acadrust::entities::EntityCommon,
viewport: Option<Handle>,
) -> &'a str {
let elt = &e.common().linetype;
let elt = &common.linetype;
if elt.is_empty() || elt.eq_ignore_ascii_case("bylayer") {
if let Some(handle) = viewport_override(
document,
&e.common().layer,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLinetypeOverride,
)
.and_then(|value| value.as_handle())
{
if let Some(line_type) = document.line_types.iter().find(|line_type| line_type.handle == handle) {
return line_type.name.as_str();
if viewport.is_some() {
if let Some(handle) = viewport_override(
document,
&common.layer,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLinetypeOverride,
)
.and_then(|value| value.as_handle())
{
if let Some(line_type) = document.line_types.iter().find(|line_type| line_type.handle == handle) {
return line_type.name.as_str();
}
}
}
document
.layers
.get(&e.common().layer)
.get(&common.layer)
.map(|l| l.line_type.as_str())
.unwrap_or("Continuous")
} else {
@@ -3404,6 +3414,14 @@ pub(in crate::scene) fn linetype_name_for_viewport<'a>(
}
}
pub(in crate::scene) fn linetype_name_for_viewport<'a>(
document: &'a CadDocument,
e: &'a EntityType,
viewport: Option<Handle>,
) -> &'a str {
linetype_name_for_common_viewport(document, e.common(), viewport)
}
/// Returns `(entity_color, pattern_length, pattern, line_weight_px, aci)` for
/// an entity, resolving ByLayer color and linetype from the document.
pub(in crate::scene) fn render_style_for(
@@ -3427,14 +3445,13 @@ fn viewport_override(
.find_map(|(handle, value)| (handle == viewport).then_some(value))
}
pub(crate) fn render_style_for_viewport(
pub(crate) fn render_style_for_common_viewport(
document: &CadDocument,
e: &EntityType,
common: &acadrust::entities::EntityCommon,
viewport: Option<Handle>,
) -> ([f32; 4], f32, [f32; 8], f32, u8) {
let layer_name = &e.common().layer;
let layer_name = &common.layer;
let (entity_color, aci) = {
let common = e.common();
let book_color = common
.color_book_handle
.filter(|handle| handle.is_valid())
@@ -3444,7 +3461,7 @@ pub(crate) fn render_style_for_viewport(
_ => None,
});
let ec = book_color.unwrap_or(&common.color);
let viewport_color = (!book_color.is_some() && *ec == AcadColor::ByLayer)
let viewport_color = (book_color.is_none() && *ec == AcadColor::ByLayer && viewport.is_some())
.then(|| {
viewport_override(
document,
@@ -3473,16 +3490,21 @@ pub(crate) fn render_style_for_viewport(
};
let [r, g, b, _] = tess_util::aci_to_rgba(resolved);
let transparency = if common.transparency.is_by_layer() {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportAlphaOverride,
)
.and_then(|value| value.as_i32())
.map(|value| acadrust::types::Transparency::from_alpha_value(value as u32))
.or_else(|| document.layers.get(layer_name).map(|layer| layer.transparency))
.unwrap_or(common.transparency)
let vp_trans = if viewport.is_some() {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportAlphaOverride,
)
.and_then(|value| value.as_i32())
.map(|value| acadrust::types::Transparency::from_alpha_value(value as u32))
} else {
None
};
vp_trans
.or_else(|| document.layers.get(layer_name).map(|layer| layer.transparency))
.unwrap_or(common.transparency)
} else {
common.transparency
};
@@ -3490,18 +3512,14 @@ pub(crate) fn render_style_for_viewport(
([r, g, b, alpha], aci)
};
let lt_name = linetype_name_for_viewport(document, e, viewport);
let lt_name = linetype_name_for_common_viewport(document, common, viewport);
// Effective scale = global LTSCALE × per-entity scale (both default to 1.0).
let lt_scale = document.header.linetype_scale as f32 * e.common().linetype_scale as f32;
let lt_scale = document.header.linetype_scale as f32 * common.linetype_scale as f32;
let (pattern_length, pattern) = resolve_pattern(&document.line_types, lt_name, lt_scale);
let line_weight_px = {
// LWDISPLAY is no longer evaluated here — the toggle is now applied in
// the wire shader via `Uniforms.lwdisplay_enable`, so we always bake the
// entity's resolved (layer-inherited) weight. Toggling lineweight
// visibility costs only a uniform write, not a retessellate.
let ew = &e.common().line_weight;
let viewport_lineweight = matches!(ew, LineWeight::ByLayer | LineWeight::Default)
let ew = &common.line_weight;
let viewport_lineweight = (viewport.is_some() && matches!(ew, LineWeight::ByLayer | LineWeight::Default))
.then(|| {
viewport_override(
document,
@@ -3526,14 +3544,29 @@ pub(crate) fn render_style_for_viewport(
(entity_color, pattern_length, pattern, line_weight_px, aci)
}
pub(crate) fn has_resolved_book_color(document: &CadDocument, e: &EntityType) -> bool {
e.common()
pub(crate) fn render_style_for_viewport(
document: &CadDocument,
e: &EntityType,
viewport: Option<Handle>,
) -> ([f32; 4], f32, [f32; 8], f32, u8) {
render_style_for_common_viewport(document, e.common(), viewport)
}
pub(crate) fn has_resolved_book_color_common(
document: &CadDocument,
common: &acadrust::entities::EntityCommon,
) -> bool {
common
.color_book_handle
.filter(|handle| handle.is_valid())
.and_then(|handle| document.objects.get(&handle))
.is_some_and(|object| matches!(object, acadrust::objects::ObjectType::BookColor(_)))
}
pub(crate) fn has_resolved_book_color(document: &CadDocument, e: &EntityType) -> bool {
has_resolved_book_color_common(document, e.common())
}
/// Resolved render style used as the inheritance source for a block child's
/// ByBlock properties (the INSERT's own style) or its layer-0 properties (the
/// INSERT's *layer* style). Bundled so it threads through the block-expansion
@@ -3569,51 +3602,71 @@ pub(crate) fn layer_render_style_viewport(
viewport: Option<Handle>,
) -> InheritStyle {
let layer = document.layers.get(layer_name);
let viewport_color = viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportColorOverride,
)
.and_then(|value| value.as_i32())
.map(AcadColor::from_true_color_value);
let has_vp = viewport.is_some();
let viewport_color = if has_vp {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportColorOverride,
)
.and_then(|value| value.as_i32())
.map(AcadColor::from_true_color_value)
} else {
None
};
let color = viewport_color
.as_ref()
.or_else(|| layer.map(|layer| &layer.color))
.unwrap_or(&AcadColor::WHITE);
let [r, g, b, _] = tess_util::aci_to_rgba(color);
let alpha = viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportAlphaOverride,
)
.and_then(|value| value.as_i32())
.map(|value| acadrust::types::Transparency::from_alpha_value(value as u32))
.or_else(|| layer.map(|layer| layer.transparency))
let alpha = if has_vp {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportAlphaOverride,
)
.and_then(|value| value.as_i32())
.map(|value| acadrust::types::Transparency::from_alpha_value(value as u32))
.or_else(|| layer.map(|layer| layer.transparency))
.map(|transparency| 1.0 - transparency.as_percent() as f32)
.unwrap_or(1.0);
let lt_name = viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLinetypeOverride,
)
.and_then(|value| value.as_handle())
.and_then(|handle| document.line_types.iter().find(|line_type| line_type.handle == handle))
.map(|line_type| line_type.name.as_str())
.or_else(|| layer.map(|layer| layer.line_type.as_str()))
.unwrap_or("Continuous");
.unwrap_or(1.0)
} else {
layer
.map(|layer| layer.transparency)
.map(|transparency| 1.0 - transparency.as_percent() as f32)
.unwrap_or(1.0)
};
let lt_name = if has_vp {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLinetypeOverride,
)
.and_then(|value| value.as_handle())
.and_then(|handle| document.line_types.iter().find(|line_type| line_type.handle == handle))
.map(|line_type| line_type.name.as_str())
.or_else(|| layer.map(|layer| layer.line_type.as_str()))
.unwrap_or("Continuous")
} else {
layer.map(|layer| layer.line_type.as_str()).unwrap_or("Continuous")
};
let lt_scale = document.header.linetype_scale as f32;
let (pat_len, pat) = resolve_pattern(&document.line_types, lt_name, lt_scale);
let viewport_lineweight = viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLineweightOverride,
)
.and_then(|value| value.as_i32())
.map(|value| LineWeight::from_value(value as i16));
let viewport_lineweight = if has_vp {
viewport_override(
document,
layer_name,
viewport,
acadrust::objects::KnownXRecordKind::LayerViewportLineweightOverride,
)
.and_then(|value| value.as_i32())
.map(|value| LineWeight::from_value(value as i16))
} else {
None
};
let lw = viewport_lineweight
.as_ref()
.or_else(|| layer.map(|layer| &layer.line_weight))