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

テクスチャ付き最大LODを出力するオプションを追加 #647

Merged
merged 48 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
1ffdbcd
lod option
satoshi7190 Aug 21, 2024
b357792
Merge branch 'main' into feature/lod-option
satoshi7190 Aug 22, 2024
9d719bc
Merge branch 'main' into feature/lod-option
satoshi7190 Aug 22, 2024
9caed2a
DataSinkProviderTest
satoshi7190 Aug 22, 2024
a98779a
Selection UI poc
satoshi7190 Aug 22, 2024
29b55ce
Change of type
satoshi7190 Aug 26, 2024
e43f931
Reflected in ui
satoshi7190 Aug 27, 2024
e9b5eb2
Replacement of naming
satoshi7190 Aug 29, 2024
9051f2d
Fixes to TransformerRegistry
satoshi7190 Aug 30, 2024
e0fb5e1
Merge branch 'main' into feature/lod-option
satoshi7190 Aug 30, 2024
c8b4e06
Add command argument.
satoshi7190 Aug 30, 2024
a4e6dc0
Additional error handling
satoshi7190 Sep 2, 2024
3079c22
Merge branch 'main' into feature/lod-option
satoshi7190 Sep 2, 2024
59589cf
Remove unnecessary imports
satoshi7190 Sep 2, 2024
1e80a45
Type modification
satoshi7190 Sep 2, 2024
0907cc1
Modification of lod options
satoshi7190 Sep 2, 2024
72ff456
Error handling fixes
satoshi7190 Sep 2, 2024
a7d9f36
Modification of naming
satoshi7190 Sep 4, 2024
dc6e722
Modification of naming
satoshi7190 Sep 4, 2024
b7006e7
Fix variable declarations
satoshi7190 Sep 4, 2024
dd99cee
Adjustments to the UI
satoshi7190 Sep 4, 2024
d7070ca
Delete requirements
satoshi7190 Sep 5, 2024
d4a4b85
Adjustment of ui
satoshi7190 Sep 5, 2024
498bcff
Added error handling of invalid commands
satoshi7190 Sep 5, 2024
cbfd15c
Comments, correction of variable names
satoshi7190 Sep 9, 2024
4c8c0e2
Implementation of early returns
satoshi7190 Sep 9, 2024
456d5c9
Modification of naming
satoshi7190 Sep 9, 2024
8fc3108
Fixing LodSelection
satoshi7190 Sep 9, 2024
3891dc9
Functionalisation of common options
satoshi7190 Sep 10, 2024
87a21ff
Remove unnecessary borrow in file_path.push() call
satoshi7190 Sep 10, 2024
ae55b39
Refactor redundant pattern matching to use is_err()
satoshi7190 Sep 10, 2024
05bc85d
Merge branch 'main' into feature/lod-option
satoshi7190 Sep 12, 2024
22ee821
Additional LOD options
satoshi7190 Sep 12, 2024
6026bd1
Correct TODO comment in Boolean parameter processing
satoshi7190 Sep 12, 2024
c449bf8
Merge branch 'main' into feature/lod-texture-option
satoshi7190 Sep 17, 2024
7aa6864
Fixing gltfsink
satoshi7190 Sep 18, 2024
56e083d
Extract textured maximum LOD.
satoshi7190 Sep 19, 2024
209fedc
Modification of the recurrence process
satoshi7190 Sep 19, 2024
9755944
Modification of processing
satoshi7190 Sep 19, 2024
f3c5812
Change of nomenclature
satoshi7190 Sep 19, 2024
d66377d
Delete unreachable patterns
satoshi7190 Sep 20, 2024
d3b2c6b
Additional option to extract all LODs
satoshi7190 Sep 20, 2024
0fbb379
Change window size
satoshi7190 Sep 23, 2024
c905271
Modify config option
satoshi7190 Sep 23, 2024
78b291f
Adjustment of ui
satoshi7190 Sep 24, 2024
1506f6d
Correction of comment-outs
satoshi7190 Sep 24, 2024
f7b3a84
Modification of command arguments
satoshi7190 Sep 24, 2024
8ed6320
Amendments to comments
satoshi7190 Sep 24, 2024
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
2 changes: 1 addition & 1 deletion app/src-tauri/tauri.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"title": "PLATEAU GIS Converter",
"fullscreen": false,
"resizable": true,
"height": 800,
"height": 900,
"width": 640,
"minHeight": 600,
"minWidth": 500
Expand Down
2 changes: 1 addition & 1 deletion app/src/routes/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
{/if}

<div class="py-5 grid place-items-center h-screen">
<div class="max-w-2xl flex flex-col gap-12 pb-8">
<div class="max-w-2xl flex flex-col gap-8 pb-4">
<div class="flex items-center gap-1.5">
<h1 class="font-bold text-2xl">PLATEAU GIS Converter</h1>
<a href="/about" class="hover:text-accent1">
Expand Down
2 changes: 1 addition & 1 deletion nusamai-plateau/src/entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use crate::appearance::AppearanceStore;

/// City objects, features, objects or data
#[derive(Debug, serde::Deserialize, serde::Serialize)]
#[derive(Debug, serde::Deserialize, serde::Serialize, Clone)]

Check warning on line 8 in nusamai-plateau/src/entity.rs

View check run for this annotation

Codecov / codecov/patch

nusamai-plateau/src/entity.rs#L8

Added line #L8 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q) 結局ここのCloneは除去できなかった感じですかね…

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

僕が提案したコードを適用すると、除去できる気はしています!

pub struct Entity {
/// Attribute tree
pub root: Value,
Expand Down
16 changes: 9 additions & 7 deletions nusamai/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@ pub fn use_lod_config(default_value: &str) -> TransformerConfig {
TransformerConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q) 前提がよく理解できていなかったんですが、option.rsはSinkのオプションで、transformer/setting.rsはトランスフォーマーのオプションを列挙している感じでしょうか?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option.rsが transformerフォルダに属していないのが気になりました。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo) optionなのか、configなのか、settingなのか…表記が揺れていて、かつどれも異なる意味を持っていそうで、理解に時間がかかっちゃいそうです…!
整理が必要かと…

key: "use_lod".to_string(),
label: "出力LODの選択".to_string(),
parameter: transformer::ParameterType::Selection(LodSelection::create_lod_selection(
default_value,
)),
parameter: transformer::ParameterType::Selection(
LodSelection::lod_selection_without_texture(default_value),
),
}
}

pub fn use_texture_config(default_value: bool) -> TransformerConfig {
pub fn use_textured_lod_config(default_value: &str) -> TransformerConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q) use_lod_configuse_textured_lod_configで、use_lodという同名キーが利用されているのが全く理解できず…
オプションは分ける必要がある感じでしょうか?

TransformerConfig {
key: "use_texture".to_string(),
label: "テクスチャの使用".to_string(),
parameter: transformer::ParameterType::Boolean(default_value),
key: "use_lod".to_string(),
label: "出力LODの選択".to_string(),
parameter: transformer::ParameterType::Selection(LodSelection::lod_selection_with_texture(
default_value,
)),
satoshi7190 marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 2 additions & 3 deletions nusamai/src/sink/cesiumtiles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use url::Url;

use crate::{
get_parameter_value,
option::{use_lod_config, use_texture_config},
option::use_textured_lod_config,
parameters::*,
pipeline::{Feedback, PipelineError, Receiver, Result},
sink::{DataRequirements, DataSink, DataSinkProvider, SinkInfo},
Expand Down Expand Up @@ -69,8 +69,7 @@ impl DataSinkProvider for CesiumTilesSinkProvider {

fn transformer_options(&self) -> TransformerRegistry {
let mut settings: TransformerRegistry = TransformerRegistry::new();
settings.insert(use_lod_config("max_lod"));
settings.insert(use_texture_config(false));
settings.insert(use_textured_lod_config("max_lod"));
satoshi7190 marked this conversation as resolved.
Show resolved Hide resolved

settings
}
Expand Down
6 changes: 2 additions & 4 deletions nusamai/src/sink/gltf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use url::Url;

use crate::{
get_parameter_value,
option::{use_lod_config, use_texture_config},
option::use_textured_lod_config,
parameters::*,
pipeline::{Feedback, PipelineError, Receiver, Result},
sink::{cesiumtiles::metadata, DataRequirements, DataSink, DataSinkProvider, SinkInfo},
Expand Down Expand Up @@ -58,12 +58,10 @@ impl DataSinkProvider for GltfSinkProvider {

fn transformer_options(&self) -> TransformerRegistry {
let mut settings: TransformerRegistry = TransformerRegistry::new();
settings.insert(use_lod_config("max_lod"));
settings.insert(use_texture_config(false));
settings.insert(use_textured_lod_config("max_lod"));

settings
}

fn create(&self, params: &Parameters) -> Box<dyn DataSink> {
let output_path = get_parameter_value!(params, "@output", FileSystemPath);
let limit_texture_resolution =
Expand Down
5 changes: 2 additions & 3 deletions nusamai/src/sink/obj/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

use crate::{
get_parameter_value,
option::{use_lod_config, use_texture_config},
option::use_textured_lod_config,
parameters::*,
pipeline::{Feedback, PipelineError, Receiver, Result},
sink::{DataRequirements, DataSink, DataSinkProvider, SinkInfo},
Expand Down Expand Up @@ -75,8 +75,7 @@

fn transformer_options(&self) -> TransformerRegistry {
let mut settings: TransformerRegistry = TransformerRegistry::new();
settings.insert(use_lod_config("max_lod"));
settings.insert(use_texture_config(false));
settings.insert(use_textured_lod_config("max_lod"));

Check warning on line 78 in nusamai/src/sink/obj/mod.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/sink/obj/mod.rs#L78

Added line #L78 was not covered by tests

settings
}
Expand Down
1 change: 1 addition & 0 deletions nusamai/src/sink/obj/obj_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ fn write_obj(

mesh_data.push((feature_id, mesh, vertex_offset, uv_offset));
}

let mut obj_writer = BufWriter::new(File::create(obj_path)?);

writeln!(obj_writer, "mtllib {}.mtl", file_name)?;
Expand Down
40 changes: 33 additions & 7 deletions nusamai/src/transformer/setting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,26 @@

impl LodSelection {
pub fn get_lod_selection_options() -> Vec<(&'static str, &'static str)> {
vec![("最大LOD", "max_lod"), ("最小LOD", "min_lod")]
vec![
("最大LOD", "max_lod"),
("最小LOD", "min_lod"),
("テクスチャ付き最大LOD", "textured_max_lod"),
// This option will be used in 3dtiles sink
// ("すべてのLOD", "all_lod"),
]
}

pub fn create_lod_selection(default_value: &str) -> Selection {
pub fn lod_selection_with_texture(default_value: &str) -> Selection {
Selection::new(Self::get_lod_selection_options(), default_value)
}

pub fn lod_selection_without_texture(default_value: &str) -> Selection {
let options = Self::get_lod_selection_options()
.into_iter()
.filter(|&(_, value)| value != "textured_max_lod")
.collect::<Vec<_>>();
Selection::new(options, default_value)
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand Down Expand Up @@ -135,10 +149,8 @@
ParameterType::String(_value) => {
// TODO: Processing for String types.
}
ParameterType::Boolean(value) => {
if config.key == "use_texture" {
data_requirements.set_appearance(*value);
}
ParameterType::Boolean(_value) => {
// TODO: Processing for Boolean types.

Check warning on line 153 in nusamai/src/transformer/setting.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/setting.rs#L152-L153

Added lines #L152 - L153 were not covered by tests
}
ParameterType::Integer(_value) => {
// TODO: Processing for Integer types.
Expand All @@ -150,14 +162,28 @@
data_requirements.set_lod_filter(transformer::LodFilterSpec {
mode: transformer::LodFilterMode::Highest,
..Default::default()
})
});
}
"min_lod" => {
data_requirements.set_lod_filter(transformer::LodFilterSpec {
mode: transformer::LodFilterMode::Lowest,
..Default::default()
})
}
"textured_max_lod" => {
data_requirements.set_lod_filter(transformer::LodFilterSpec {
mode: transformer::LodFilterMode::TexturedHighest,
..Default::default()
});
data_requirements.set_appearance(true);
}
"all_lod" => {
data_requirements.set_lod_filter(transformer::LodFilterSpec {
mode: transformer::LodFilterMode::All,
..Default::default()
});
data_requirements.set_appearance(true);
}

Check warning on line 186 in nusamai/src/transformer/setting.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/setting.rs#L173-L186

Added lines #L173 - L186 were not covered by tests
satoshi7190 marked this conversation as resolved.
Show resolved Hide resolved
_ => {}
}
}
Expand Down
76 changes: 65 additions & 11 deletions nusamai/src/transformer/transform/lods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
pub enum LodFilterMode {
Highest,
Lowest,
TexturedHighest,
All,
}

#[derive()]
Expand All @@ -29,16 +31,68 @@
/// Transform to filter and split the LODs
impl Transform for FilterLodTransform {
fn transform(&mut self, _feedback: &Feedback, mut entity: Entity, out: &mut Vec<Entity>) {
let lods = find_lods(&entity.root) & self.mask;

let target_lod = match self.mode {
LodFilterMode::Highest => lods.highest_lod(),
LodFilterMode::Lowest => lods.lowest_lod(),
};

if let Some(target_lod) = target_lod {
edit_tree(&mut entity.root, target_lod);
out.push(entity);
match self.mode {
LodFilterMode::TexturedHighest => {
Copy link
Collaborator

@nokonoko1203 nokonoko1203 Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo) テクスチャが付いた最大のLODを抽出する。テクスチャがない場合は最大のLODを抽出する。
くらいのコメントはあった方が良いと思いました!

// TODO: Processing needs to be optimised
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits) Processing needs to be optimized.
nits) 「最適化」とは具体的に何をするのか、記載しておいた方が良いかと!

let original_lods = find_lods(&entity.root) & self.mask;
Copy link
Collaborator

@nokonoko1203 nokonoko1203 Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi) シンプルに、こんな感じではいかがでしょう。
一応、自分で動かしてみたところ、動作はしているように見えました。

let available_lods = find_lods(&entity.root) & self.mask;
let mut highest_textured_lod = None;

// 「最大のLOD」は初めから決まっている。テクスチャが存在しなければ、即座に最大LODを返却できる
let highest_available_lod = available_lods.highest_lod().unwrap_or(0);

// revで逆順イテレータを作成
for lod in (0..=highest_available_lod).rev() {
	if available_lods.0 & (1 << lod) != 0 {
		edit_tree(&mut entity.root, lod);

		let has_textures = {
			let appearance = entity.appearance_store.read().unwrap();
			!appearance.textures.is_empty()
		};

		// テクスチャが存在するLODが見つかれば、それを保存して終了
		if has_textures {
			highest_textured_lod = Some(lod);
			break;
		}
	}
}

// highest_textured_lodがNoneではないならhighest_textured_lodを利用
// Noneならhighest_available_lodを利用
if let Some(lod) = highest_textured_lod.or(Some(highest_available_lod)) {
	edit_tree(&mut entity.root, lod);
	out.push(entity);
}
Suggested change
let original_lods = find_lods(&entity.root) & self.mask;
let original_lods = find_lods(&entity.root) & self.mask;

let mut current_lods = original_lods;
let mut highest_lod_entity = None;

Check warning on line 39 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L37-L39

Added lines #L37 - L39 were not covered by tests

while current_lods.0 != 0 {
let target_lod = current_lods.highest_lod();

Check warning on line 42 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L41-L42

Added lines #L41 - L42 were not covered by tests

if let Some(lod) = target_lod {

Check warning on line 44 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L44

Added line #L44 was not covered by tests
// Create a copy of the entity
let mut entity_copy = entity.clone();
edit_tree(&mut entity_copy.root, lod);

let has_textures = {
let appearance = entity_copy.appearance_store.read().unwrap();
!appearance.textures.is_empty()
};
satoshi7190 marked this conversation as resolved.
Show resolved Hide resolved

if has_textures {
out.push(entity_copy);
return;

Check warning on line 56 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L46-L56

Added lines #L46 - L56 were not covered by tests
} else {
// Save the highest LOD entity
if highest_lod_entity.is_none() {
highest_lod_entity = Some(entity_copy);
}

Check warning on line 61 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L59-L61

Added lines #L59 - L61 were not covered by tests
// Exclude the current LOD and try the next LOD
current_lods.0 &= !(1 << lod);

Check warning on line 63 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L63

Added line #L63 was not covered by tests
}
} else {
break;

Check warning on line 66 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L66

Added line #L66 was not covered by tests
}
}

// If no texture is found, push entity with highest LOD
if let Some(highest_entity) = highest_lod_entity {
out.push(highest_entity);
}

Check warning on line 73 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L71-L73

Added lines #L71 - L73 were not covered by tests
satoshi7190 marked this conversation as resolved.
Show resolved Hide resolved
}
LodFilterMode::Highest => {
let lods = find_lods(&entity.root) & self.mask;
let target_lod = lods.highest_lod();

if let Some(target_lod) = target_lod {
edit_tree(&mut entity.root, target_lod);
out.push(entity);
}
}
LodFilterMode::Lowest => {
let lods = find_lods(&entity.root) & self.mask;
let target_lod = lods.lowest_lod();

Check warning on line 86 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L85-L86

Added lines #L85 - L86 were not covered by tests

if let Some(target_lod) = target_lod {
edit_tree(&mut entity.root, target_lod);
out.push(entity);
}

Check warning on line 91 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L88-L91

Added lines #L88 - L91 were not covered by tests
}
LodFilterMode::All => {
out.push(entity);
}

Check warning on line 95 in nusamai/src/transformer/transform/lods.rs

View check run for this annotation

Codecov / codecov/patch

nusamai/src/transformer/transform/lods.rs#L93-L95

Added lines #L93 - L95 were not covered by tests
}
}

Expand Down Expand Up @@ -92,7 +146,7 @@
mask
}

#[derive(Default, Clone, Copy)]
#[derive(Default, Clone, Copy, Debug)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits) Debugは消しときましょうか!

pub struct LodMask(
u8, // lods bit mask
);
Expand Down