Reflected Ceiling Plan

[theoryshaw]

Didn't see an issue yet created for this. :)

Not sure, would just mirroring the svg work? But not the text.

[Moult]

@aothms thoughts on this one? In Blender I can visualise an RCP in the viewport by scaling the camera X axis by -1, and in theory I could do do a transform scale(1,-1), translate(0,-drawingheight) in SVG, but should this change how the drawing geometry is defined in IFC?

[aothms]

and in theory I could do do a transform scale(1,-1), translate(0,-drawingheight)

We already have this in SVG actually in how the geometries are fed to the serializer, because the coordinate system in SVG starts at the top with positive axis downwards. So implementing this in C++ (bypassing the mirror we do now) is actually fairly trivial with some flag. In theory, because there's quite a few bits and pieces where this may bite us (for example in draw.py when we trace back the cells to the ifc elements).

Another option maybe is never to really bother with this and until we actually place the drawing on a sheet?

[Moult]

How does placing it on a sheet change things? The user should see the RCP as is in the viewport so that they can place text tags etc, right?

[aothms]

Hm, not sure, if we're talking about the same thing, but text is indeed an issue, you can't mirror the entire drawing as is without having the text literals also mirror. But that applies similarly to your scale(1,-1), translate(0,-drawingheight) which I assumed was on some sort of upper group. So we probably do need to instruct the serializer to disable mirroring on the geometry.

[Moult]

Yeah it'll be on the group that contains the output from the serializer. The text and other annotations right now are still handled in non ifcconvert code (I.e. blender python) so I can handle those transformations myself.

The user first sees the RCP in the blender viewport (we can mirror the blender camera), then they will generate the drawing which has multiple optional layers (a mirrored raster layer rendered from blender, the serializer linework output, and the annotation output from blenderbim). Then, that group is placed on a sheet as an embedded svg. Finally, that sheet might be compiled into a single file svg with all embeds inlined for distribution.

So the transform I was referring to would be on the serializer line work output layer.

But yes, would be much, much better if this could be a toggle in the serializer.

[Moult]

Ping @aothms any chance to look at this one yet?

[Moult]

Bump @aothms any chance of adding this flag to support an RCP? https://community.osarch.org/discussion/1522/does-anyone-have-a-hacky-workflow-to-create-a-rcp#latest

[aothms]

Sorry for the delay. I'm worried I would mess this up and waiting for the openbot builds to test this iteratively seems extremely painful. Is there a way you make these changes locally first and test and I just write a patch or sth?

[Moult]

@aothms sure I'm happy to play around myself and try compile it locally. I haven't had a chance to look at this portion of the code yet - can you help point me to the portions to look at?

[aothms]

I would start with something like this:

diff --git a/src/serializers/SvgSerializer.cpp b/src/serializers/SvgSerializer.cpp
index cdeb38384..5db5c2ebd 100644
--- a/src/serializers/SvgSerializer.cpp
+++ b/src/serializers/SvgSerializer.cpp
@@ -785,7 +785,9 @@ void SvgSerializer::write(const geometry_data& data) {
        // SVG has a coordinate system with the origin in the *upper*-left corner
        // therefore we mirror the shape along the XZ-plane.
        gp_Trsf trsf_mirror;
-       trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY()));
+       if (!no_mirror_) {
+               trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY()));
+       }
        BRepBuilderAPI_Transform make_transform_mirror(compound_unmirrored, trsf_mirror, true);
        make_transform_mirror.Build();
        // (When determinant < 0, copy is implied and the input is not mutated.)
@@ -1797,7 +1799,9 @@ void SvgSerializer::draw_hlr(const gp_Pln& pln, const drawing_key& drawing_name)
                TopoDS_Shape hlr_compound;
                if (drawing_name.first == nullptr) {
                        gp_Trsf trsf_mirror;
-                       trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY()));
+                       if (!no_mirror_) {
+                               trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY()));
+                       }
                        BRepBuilderAPI_Transform make_transform_mirror(hlr_compound_unmirrored, trsf_mirror, true);
                        make_transform_mirror.Build();
                        hlr_compound = make_transform_mirror.Shape();
diff --git a/src/serializers/SvgSerializer.h b/src/serializers/SvgSerializer.h
index b5cdc64d6..427e7ac76 100644
--- a/src/serializers/SvgSerializer.h
+++ b/src/serializers/SvgSerializer.h
@@ -166,6 +166,11 @@ protected:
        bool emit_building_storeys_;
        bool no_css_;

+       // The coordinate system in SVG does not correspond to that in IFC because in
+       // SVG the origin is in the top-left and positive Y is downwards over the screen.
+       // Hence, element geometry is mirrored unless this settings is set to true.
+       bool no_mirror_;
+
        int profile_threshold_;

        IfcParse::IfcFile* file;
@@ -215,6 +220,7 @@ public:
                , polygonal_(false)
                , emit_building_storeys_(true)
                , no_css_(false)
+               , no_mirror_(false)
                , profile_threshold_(-1)
                , file(0)
                , storey_(0)
@@ -338,6 +344,14 @@ public:
                return profile_threshold_;
        }

+       void setNoMirror(bool b) {
+               no_mirror_ = b;
+       }
+
+       bool getNoMirror() const {
+               return no_mirror_;
+       }
+
 protected:
        std::string writeMetadata(const drawing_meta& m);
 };

That will undo the mirroring over Y to match coordinate system defaults. If that's not as intended (you want to mirror over X instead) change to:

trsf_mirror.SetMirror(gp_Ax2(gp::Origin(), gp::DY()));
if (mirror_x_) { // same idea different variable name
    gp_Trsf mirror_x;
    mirror_x.SetMirror(gp_Ax2(gp::Origin(), gp::DX()));
    trsf_mirror.PreMultiplt(mirror_x);
}

[theoryshaw]

Trying to work out a hack. :)
https://community.osarch.org/discussion/comment/16303/#Comment_16303

[Moult]

@aothms I've finally gotten around to testing the snippet and I can get it to work with either X or Y mirroring (though it seems X is more "intuitive" in the unreflected view) but the issue is that even though the shapes are mirrored correctly, they now appear off canvas. Any ideas where to look in the code? Happy to poke around.

Edit: did a slight poke around and seems to me it's probably the resize() function that needs tweaking but I'll investigate more later when I find the time.

This is how I send data to IfcOpenShell's svg serialiser:

And this is what I expect to come back after mirroring x:

But I get this instead:

[Moult]

Managed to fix it with:

+if (mirror_x_) {
+    cx = - size_->first - cx;
+}
m = {{ {{sc,0,-cx}},{{0,sc,-cy}},{{0,0,1}} }};

In resize(). Is that the right approach or did I mess up?

[aothms]

🎉 Great. Glad we're making progress.

Regarding the offset. Honestly I have a hard time reconstructing the mental model behind this plate of spaghetti, if it works it works I'd say.

It could also be that you can fix it here:

IfcOpenShell/src/serializers/SvgSerializer.cpp

Lines 675 to 676 in 4c8007d

	((-size->first / 2.) - pu) * 1000 * *scale_,
	((-size->second / 2.) + pv) * 1000 * *scale_

Notice we do - pu but + pv. Maybe for mirror_x we need to do + pu as well? At least I have some vague intuition that the distinction - vs + was due to mirroring (which we by default only do for Y axis, because of the coordinate system differences between svg and ifc).

[Moult]

Cheers! It took a few tries, a lot of std::cout to see what the variables were holding, and a little paper diagram to figure it out :) I didn't fix it there because that seemed to only affect offset_2d_ which is only in one scenario:

IfcOpenShell/src/serializers/SvgSerializer.cpp

Line 1650 in 4c8007d

if (offset_2d_ && scale_) {

[aothms]

cout is very painful in C++ due to the recompilation. next time we're on a call I'll show you a couple minutes of gdb so that you can attach and inspect running processes.

[theoryshaw]

I'm excited to try this.
Thanks Gents.