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.

Sign up for GitHub

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

Jump to bottom

Retain loaded position of an imported obj file #283

Closed
wants to merge 3 commits into from
Closed

Retain loaded position of an imported obj file #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ccef95
Restore OBJ Importer so that importing 3D objects works again
mvriel Oct 7, 2024
2aaa133
Refactor the positioning script to use coordinates fully, to ensure n…
mvriel Oct 7, 2024
c5ab670
Make sure the position of an imported 3d model doesn't overwrite a po…
mvriel Oct 8, 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
56 changes: 56 additions & 0 deletions Assets/Prefabs/ObjImporter.prefab
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions Assets/Prefabs/ObjImporter.prefab.meta
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

89 changes: 53 additions & 36 deletions Assets/Scripts/Layers/LayerTypes/HierarchicalObjectLayerGameObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using Netherlands3D.Coordinates;
using Netherlands3D.Twin.FloatingOrigin;
using Netherlands3D.Twin.Layers.LayerTypes;
using Netherlands3D.Twin.Layers.Properties;
using Netherlands3D.Twin.Projects;
Expand All @@ -11,22 +12,28 @@

namespace Netherlands3D.Twin.Layers
{
[RequireComponent(typeof(WorldTransform))]
public class HierarchicalObjectLayerGameObject : LayerGameObject, IPointerClickHandler, ILayerWithPropertyPanels, ILayerWithPropertyData
{
private ToggleScatterPropertySectionInstantiator toggleScatterPropertySectionInstantiator;
[SerializeField] private UnityEvent<GameObject> objectCreated = new();
private List<IPropertySectionInstantiator> propertySections = new();
private TransformLayerPropertyData transformPropertyData;
private Vector3 previousPosition;
private Coordinate previousPosition;
private Quaternion previousRotation;
private Vector3 previousScale;
private WorldTransform worldTransform;

LayerPropertyData ILayerWithPropertyData.PropertyData => transformPropertyData;

protected void Awake()
{
var coord = new Coordinate(transform.position);
transformPropertyData = new TransformLayerPropertyData(coord, transform.eulerAngles, transform.localScale);
worldTransform = GetComponent<WorldTransform>();
transformPropertyData = new TransformLayerPropertyData(
worldTransform.Coordinate,
worldTransform.Rotation.eulerAngles,
transform.localScale
);

propertySections = GetComponents<IPropertySectionInstantiator>().ToList();
toggleScatterPropertySectionInstantiator = GetComponent<ToggleScatterPropertySectionInstantiator>();
Expand All @@ -47,8 +54,8 @@ protected override void OnDisable()
protected override void Start()
{
base.Start();
previousPosition = transform.position;
previousRotation = transform.rotation;
previousPosition = worldTransform.Coordinate;
previousRotation = worldTransform.Rotation;
previousScale = transform.localScale;

objectCreated.Invoke(gameObject);
Expand All @@ -68,62 +75,72 @@ protected void OnDestroy()

private void UpdatePosition(Coordinate newPosition)
{
if (newPosition.ToUnity() != transform.position)
transform.position = newPosition.ToUnity();
if (newPosition.ToUnity() == worldTransform.Coordinate.ToUnity()) return;

worldTransform.Coordinate = newPosition;
transform.position = worldTransform.Coordinate.ToUnity();
}

private void UpdateRotation(Vector3 newAngles)
{
if (newAngles != transform.eulerAngles)
transform.eulerAngles = newAngles;
if (newAngles == transform.eulerAngles) return;

worldTransform.Rotation = Quaternion.Euler(newAngles);
transform.rotation = worldTransform.Rotation;
Copy link
Contributor

Choose a reason for hiding this comment

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

gaat dit goed met de bolling van de aarde? moet er niet ook een ToUnity() functie worden aangeroepen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

worldTransform.Rotation is een Quaternion die -als ik @mvangog goed begrepen heb- de rotatie van de gameobject is na toepassen van de bolling

Copy link
Contributor

Choose a reason for hiding this comment

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

Maar dan zou de gebolde rotatie nu toepast worden op het gameObject, waardoor een object op de evenaar dus 90º gedraaid zou zijn?

Copy link
Contributor Author

@mvriel mvriel Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🤔 Ik zou momenteel niet zo goed weten hoe dit te testen; we doen nu nog alles met RD waardoor de bolling niet wordt toegepast..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En anders draai ik de rotatie dingen even terug; voor de zekerheid

Copy link
Contributor

Choose a reason for hiding this comment

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

worldtransform.rotation is de rotatie van het gameobject in zijn eigen coordinatenstelsel. (dus vóór het toepassen van de "bolling", zoals mike het noemt)
de rotatie in unity wordt uitgerekend met worldtransform.coordinate.RotationToUnityUp() * worldtransform.rotation

}

private void UpdateScale(Vector3 newScale)
{
if (newScale != transform.localScale)
transform.localScale = newScale;
if (newScale == transform.localScale) return;

transform.localScale = newScale;
}

public virtual void LoadProperties(List<LayerPropertyData> properties)
{
var transformProperty = (TransformLayerPropertyData)properties.FirstOrDefault(p => p is TransformLayerPropertyData);
if (transformProperty != null)
var transformProperty = properties.OfType<TransformLayerPropertyData>().FirstOrDefault();
if (transformProperty == default(TransformLayerPropertyData)) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

waarom niet expliciet transformProperty == null hier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Als je in properties.OfType<TransformLayerPropertyData>().FirstOrDefault() 'ofType' gebruikt dan heb je het voordeel dat de conversie naar TransformLayerPropertyData automatisch goed gaat, dus heb je geen casting meer nodig, maar het betekent wel dat FirstOrDefault niet perse een null value teruggeeft, maar de 'default' van TransformLayerPropertyData. Dus als je FirstOrDefault gebruikt, dan is het veiliger om default(...) te gebruiken zodat je zeker weet dat de check klopt (en default is regelmatig null, maar niet altijd; zoals bij structs IIRC)

Copy link
Contributor

Choose a reason for hiding this comment

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

ja, default is alleen null als de Type nullable is.

maar voor mij geeft een == null aan dat er niks is gevonden, en hoe ik dit lees is dat er een default waarde is gevonden, wat niet per se null hoeft te zijn.

verder is dit geen blokkerende issue, maar ik vroeg me gewoon af wat het voordeel hiervan was, al begrijp ik jouw uitleg nog niet helemaal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FirstOrDefault geeft geen null terug als die niks vind, die geeft -zoals de methode naam aangeeft- de default terug als die niks vind. Toevallig is dat voor nullable type 'null'; maar het is met FirstOrDefault vaak beter -zeker in combinatie met ofType omdat dan default daadwerkelijk over die type gaat- om te checken op default(type).

Het verschil zit hem in dit ofType als je die niet gebruikt, dan weet Linq niet welk type resultaat eruit gaat komen, dus dan is default 'null'. Maar als je ofType gebruikt, dan krijg je automatische typecasting, maar dus ook van je default; en dus moet je checken op default(type) en niet op null


// unsubscribe events from previous property object, resubscribe to new object at the end of this if block
if (transformPropertyData != null)
{
if (transformPropertyData != null) //unsubscribe events from previous property object, resubscribe to new object at the end of this if block
{
transformPropertyData.OnPositionChanged.RemoveListener(UpdatePosition);
transformPropertyData.OnRotationChanged.RemoveListener(UpdateRotation);
transformPropertyData.OnScaleChanged.RemoveListener(UpdateScale);
}

this.transformPropertyData = transformProperty; //take existing TransformProperty to overwrite the unlinked one of this class

UpdatePosition(this.transformPropertyData.Position);
UpdateRotation(this.transformPropertyData.EulerRotation);
UpdateScale(this.transformPropertyData.LocalScale);

transformPropertyData.OnPositionChanged.AddListener(UpdatePosition);
transformPropertyData.OnRotationChanged.AddListener(UpdateRotation);
transformPropertyData.OnScaleChanged.AddListener(UpdateScale);
transformPropertyData.OnPositionChanged.RemoveListener(UpdatePosition);
transformPropertyData.OnRotationChanged.RemoveListener(UpdateRotation);
transformPropertyData.OnScaleChanged.RemoveListener(UpdateScale);
}

// take existing TransformProperty to overwrite the unlinked one of this class
this.transformPropertyData = transformProperty;

UpdatePosition(this.transformPropertyData.Position);
UpdateRotation(this.transformPropertyData.EulerRotation);
UpdateScale(this.transformPropertyData.LocalScale);

// Reset the previous[X] to prevent a reset of the
previousPosition = transformPropertyData.Position;
previousRotation = Quaternion.Euler(transformPropertyData.EulerRotation);
previousScale = transformPropertyData.LocalScale;

transformPropertyData.OnPositionChanged.AddListener(UpdatePosition);
transformPropertyData.OnRotationChanged.AddListener(UpdateRotation);
transformPropertyData.OnScaleChanged.AddListener(UpdateScale);
}

private void Update()
{
// We cannot use transform.hasChanged, because this flag is not correctly set when adjusting this transform using runtimeTransformHandles, instead we have to compare the values directly
// Check for position change
if (transform.position != previousPosition)
if (worldTransform.Coordinate.ToUnity() != previousPosition.ToUnity())
Copy link
Contributor

Choose a reason for hiding this comment

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

kan er niet een directe coordinate vergelijking gedaan worden (of worden toegevoegd)? dit lijkt me een beetje vatbaar voor floating point errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

De Coordinate class heeft geen Equality operator implementatie; daar liep ik tegenaan toen ik dit stuk omschreef. Maar ik wilde de scope ook niet heel erg vergroten door operator overrides in de Coordinate converter package toe te voegen

{
var rdCoordinate = new Coordinate(CoordinateSystem.Unity, transform.position.x, transform.position.y, transform.position.z);
transformPropertyData.Position = rdCoordinate;
previousPosition = transform.position;
transformPropertyData.Position = worldTransform.Coordinate;
previousPosition = worldTransform.Coordinate;
}

// Check for rotation change
if (transform.rotation != previousRotation)
if (worldTransform.Rotation != previousRotation)
{
transformPropertyData.EulerRotation = transform.eulerAngles;
previousRotation = transform.rotation;
transformPropertyData.EulerRotation = worldTransform.Rotation.eulerAngles;
previousRotation = worldTransform.Rotation;
}

// Check for scale change
Expand Down
21 changes: 9 additions & 12 deletions Assets/Scripts/Layers/LayerTypes/ObjSpawner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand All @@ -20,6 +21,11 @@ public class ObjSpawner : MonoBehaviour, ILayerWithPropertyData

private ObjImporter.ObjImporter importer;

private void Awake()
{
gameObject.transform.position = ObjectPlacementUtility.GetSpawnPoint();
}

private void Start()
{
StartImport();
Expand Down Expand Up @@ -68,8 +74,10 @@ private void ImportObj(string path)

private void OnObjImported(GameObject returnedGameObject)
{
// By explicitly stating the worldPositionStays to false, we ensure Obj is spawned and it will retain the
// position and scale in this parent object
returnedGameObject.transform.SetParent(this.transform, false);
AddLayerScriptToObj(returnedGameObject);
returnedGameObject.AddComponent<MeshCollider>();

DisposeImporter();
}
Expand All @@ -78,16 +86,5 @@ private void DisposeImporter()
{
if (importer != null) Destroy(importer.gameObject);
}

private void AddLayerScriptToObj(GameObject parsedObj)
{
var spawnPoint = ObjectPlacementUtility.GetSpawnPoint();

gameObject.transform.position = spawnPoint;

parsedObj.AddComponent<MeshCollider>();

// CreatedMoveableGameObject.Invoke(parsedObj);
}
}
}
9 changes: 6 additions & 3 deletions Assets/Scripts/ObjectPlacementUtility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@ public static class ObjectPlacementUtility
public static Vector3 GetSpawnPoint()
{
var camera = Camera.main;
var ray = camera.ScreenPointToRay(new Vector2(Screen.width / 2, Screen.height / 2));
var cameraTransform = camera.transform;

var ray = camera.ScreenPointToRay(new Vector2(Screen.width / 2f, Screen.height / 2f));
var plane = new Plane(Vector3.up, 0);
var intersect = plane.Raycast(ray, out float distance);
if (!intersect)
{
distance = 10f;
}

var spawnPoint = camera.transform.position + camera.transform.forward * distance;
return spawnPoint;
return cameraTransform.position + cameraTransform.forward * distance;
}
}
}
14 changes: 4 additions & 10 deletions Assets/_BuildingBlocks/FloatingOrigin/Scripts/WorldTransform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,10 @@ public class WorldTransform : MonoBehaviour, IHasCoordinate
{
[SerializeField] private WorldTransformShifter worldTransformShifter;
[SerializeField] private CoordinateSystem referenceCoordinateSystem = CoordinateSystem.RDNAP;
public Coordinate Coordinate {
get;
set;
}
public Quaternion Rotation
{
get;
set;
}

public Coordinate Coordinate { get; set; } = new(CoordinateSystem.RDNAP);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ik meen me te herinneren dat deze constructor kapot was. Kan je even met Martijn dubbelchecken of dat opgelost is? Ik heb hier een keer uren aan besteed omdat ik niet begreep wat er fout was.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, ik heb geen problemen ervaren bij testen, maar goed dat je dat dan aangeeft. @mvangog: weet jij dit toevallig nog?

Copy link
Contributor

@mvangog mvangog Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

het coordinaat wrdt nu geinitiliseerd as rdnap (is 3d) met 0 waarden. het gevaar hierbij is dat, als deze om een of andere reden gebruikt gaat worden met transformaties, er rare dingen gaan gebeuren omdat er 3 waarden aanwezig horen te zijn. (dat is volgens mij het probleem waar Tom toen tegenaan liep, of het was rdnap met 2 waarden)
ik zou hier een new coordinate(0,0,coordinateSystem.Undefined) doen. de transformatie-tooling weet dat ie niets kan met coordaintesystem.undefined.


public Quaternion Rotation { get; set; } = Quaternion.identity;
public CoordinateSystem ReferenceCoordinateSystem => referenceCoordinateSystem;
public Origin Origin => Origin.current;

Expand Down Expand Up @@ -56,7 +51,6 @@ private void OnEnable()
{
ShiftTo(Coordinate, Coordinate);
}

}

private void OnDisable()
Expand Down
Loading