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

Feature: Optional scale and rotation arguments for SaveAsWexBim #559

Merged

Conversation

miseifert
Copy link
Contributor

The SaveAsWexBim method already allowed for translation but not for an optional scaling and rotation.

The scaling has to be a scalar value because otherwise the value for OneMeter that is written to the file would be ambiguous.


The use-case here is that I want to scale a model to meters and re-apply the rotation (that I lost because I also need to set adjustWcs: true because it has a large offset placement and it's in millimeters) before I display it in the browser, together with other models.

But it feels impossible to get a rotation and scale applied without a change in the library itself (or re-implementing the SaveAsWexBim logic myself)

Basically what I want to do in my code is something along these lines:

using (var model = IfcStore.Open(fileName))
{
	var context = new Xbim3DModelContext(model);
	context.CreateContext(adjustWcs: true);  // without adjusting the geometry loses a lot of precision because of floating-point precision...

	// Get the matrix representing the placement if we didn't have adjustWcs
	var originalPlacement = model.Instances.OfType<IIfcLocalPlacement>(activate: true)
		.Where(lp => lp.PlacementRelTo == null)
		.Single()
		.RelativePlacement
		.ToMatrix3D();
	var rotation = originalPlacement.GetRotationQuaternion();  // get the rotation part
	var customScale = model.ModelFactors.LengthToMetresConversionFactor;
		
	var wexBimFilename = Path.ChangeExtension(fileName, "wexBIM");
	using (var wexBiMfile = File.Create(wexBimFilename))
	using (var wexBimBinaryWriter = new BinaryWriter(wexBiMfile))
	{
		model.SaveAsWexBim(wexBimBinaryWriter, scale: customScale, rotation: rotation);
		wexBimBinaryWriter.Close();
	}
	wexBiMfile.Close();
}

Sorry, I totally missed the part in the contributor guidelines that I should create an issue first. After the work I put in the commit and PR description I thought I will try it with the PR (and this apology 🙏 ). That's my bad and if you want to gauge the interest with an issue first please let me know and I will close the PR and open an issue instead.

The SaveAsWexBim method already allowed for translation but not for an optional scaling and rotation.

The scaling has to be a scalar value because otherwise the value for OneMeter that is written to the file would be ambiguous.
@andyward andyward requested a review from martin1cerny April 15, 2024 21:47
@andyward
Copy link
Member

Thanks for this - and the update. It looks sensible non-breaking and green lights on tests is good news. I've added one minor review item. I'd like @martin1cerny to review for any WCS / wexbim Region issues as that's his domain. Other than than good to merge. Cheers!

For scale it avoids coalescing the nullable value multiple times

Signed-off-by: Michael Seifert-Eckert <[email protected]>
@martin1cerny martin1cerny merged commit 7814185 into xBimTeam:develop May 29, 2024
1 check passed
@martin1cerny
Copy link
Member

Thank you for the contribution @miseifert. It is much appreciated!

@miseifert miseifert deleted the feature/scale-rotation-for-wexbim branch May 31, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants