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

Issue 695 - Milestone 5: Autocalibration #699

Merged
merged 37 commits into from
Oct 9, 2024
Merged

Issue 695 - Milestone 5: Autocalibration #699

merged 37 commits into from
Oct 9, 2024

Conversation

sebjf
Copy link
Contributor

@sebjf sebjf commented Sep 17, 2024

This fixes #695

Description

This PR extends the DGN and DWG importers to write the horizontal calibration of a drawing when processDrawing is called. The calibration is always written and contains a pair of a pair of vectors from which the calibration between SVG coordinates and Repo/Project coordinates can be recovered.

This PR also increase the minimum line with from 0.01 to 0.08.

The line that does this for further tweaks is:

dev->properties()->putAt(L"MinimalWidth", OdRxVariantValue(0.08));

@carmenfan carmenfan self-assigned this Sep 17, 2024
Copy link
Member

@carmenfan carmenfan left a comment

Choose a reason for hiding this comment

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

@sebjf as mentioned in the meeting, I think we can do with darkening the colours a bit. The thickness is good now but the colour still makes it quite faint on white background

another thing.. I've tried exporting a DWG as per your comment with shared coordinates and it still didn't get auto calibrated... am I missing something?
image
File here: https://asitesol-my.sharepoint.com/:u:/g/personal/cfan_asite_com/Ee8hosNXoE9Hm-8rwEcu-HwBr5DFdwQr4Qy-lvgarK5E-Q?e=NfhfyV

bouncer/src/repo/core/model/bson/repo_bson_factory.cpp Outdated Show resolved Hide resolved
bouncer/src/repo/core/model/repo_model_global.h Outdated Show resolved Hide resolved
@carmenfan
Copy link
Member

@sebjf the shared coords version rotates the image which is super weird...
image

would it be possible to make sure it's always upright and the rotation is just considered in the calibration?

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

Hi @carmenfan,

@sebjf the shared coords version rotates the image which is super weird...

This appears to be a Revit thing, as the DWG in AutoCAD is rotated relative to the WCS:

image

(I can further confirm by checking the coordinates of a straight line and seeing that it changes in both.)

That is, this isn't part of the view, but baked into the geometric elements, so we can't undo it as we don't know what the rotation would be.
However, I expect that drawings like this would not be produced, as those working on them would also be annoyed to see them rotated in AutoCAD as well.

(I'd guess that the Survey Points have been set but the North angle has been set to something odd to result in that export.)

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

and it still didn't get auto calibrated

Is there a branch where the autocalibration should be used now that I can test on too?

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

I think we can do with darkening the colours a bit.

I have had a look into this. It doesn't appear as if there is a straightforward way to do it yet.

In AutoCAD colours can be defined from Colour Blocks, ACI (index) or True Colour, also, elements can have colours set per layer, block or directly.

So, we could change the ACI for example, but that wouldn't change elements using True Colour or Colour Blocks. Though, if we can assume most colours are set by the ACI, that could work...

I am also looking to see if we can get the SVG exporter to make everything grayscale, though no luck yet.

Another possibility occurs to me though:

Santiago already munges the SVG to apply a background. We could pre-process the SVG further to darken all the colours. It should not be hard to iterate over all the elements which define a colour and apply a transform to it. We could also then change the behaviour too, for example, darken lines but not fills, or make it optional.

If you want I can look at doing that?

@carmenfan
Copy link
Member

@sebjf I'm not too familiar with colours so I might be saying something really dumb here (if so forgive me 😆 )

Could we not do something like halving the rgb values?

Santiago already munges the SVG to apply a background. We could pre-process the SVG further to darken all the colours. It should not be hard to iterate over all the elements which define a colour and apply a transform to it. We could also then change the behaviour too, for example, darken lines but not fills, or make it optional.

Would this apply when we put that svg into the 3D? Not sure where abouts does Santiago do this... If we investigate this approach we'd need to make sure we're not making PDF imported ons any less clear

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

Hi @carmenfan,

Could we not do something like halving the rgb values?

Yes we can do that, the question is just where its done; with the SVG export, the exporter appears as a device we hand a drawing database, so we don't iterate over any of the elements ourselves in bouncer, so there's no obvious place to update the colours. We'd have to explicitly do that, and account for all different ways colours are assigned.

We could maybe update the ACI map assigned to the database, but that only covers some of the cases.

Another possibility is to look at modifying the SVG exporter. I believe we have the code for this from ODA, so we could indeed just make a version of it that applies a scalar to whatever colour triples are written, right before it does so.

Alternatively, we could iterate over all the elements in the svg xml at runtime, looking for those that have a stroke or fill property, and halve them there.

Would this apply when we put that svg into the 3D?

Santiago can confirm but I believe it would.

If we investigate this approach we'd need to make sure we're not making PDF imported ons any less clear

Hmmm yes it would apply to the PDFs too. We could implement hueristics, such as, if the colour is already below a certain luminance, then don't do anything to it...

@carmenfan
Copy link
Member

Hi @carmenfan,

Could we not do something like halving the rgb values?

Yes we can do that, the question is just where its done; with the SVG export, the exporter appears as a device we hand a drawing database, so we don't iterate over any of the elements ourselves in bouncer, so there's no obvious place to update the colours. We'd have to explicitly do that, and account for all different ways colours are assigned.

We could maybe update the ACI map assigned to the database, but that only covers some of the cases.

Another possibility is to look at modifying the SVG exporter. I believe we have the code for this from ODA, so we could indeed just make a version of it that applies a scalar to whatever colour triples are written, right before it does so.

Alternatively, we could iterate over all the elements in the svg xml at runtime, looking for those that have a stroke or fill property, and halve them there.

Would this apply when we put that svg into the 3D?

Santiago can confirm but I believe it would.

If we investigate this approach we'd need to make sure we're not making PDF imported ons any less clear

Hmmm yes it would apply to the PDFs too. We could implement hueristics, such as, if the colour is already below a certain luminance, then don't do anything to it...

@sebjf My instinct would be that we should do it on the exporter side so we are only dealing with DWGs, the PDF looks good we don't really want to interfer with it and make assumptions.

But if it's easier to the latter then I'm open to trying

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

I've just looked at the source for the svg exporter and they have a colour policy that can make everything monochrome, but its not in the documentation anywhere 🤦‍♂️ . I will see if i can activate it in our version...

@carmenfan
Copy link
Member

I've just looked at the source for the svg exporter and they have a colour policy that can make everything monochrome, but its not in the documentation anywhere 🤦‍♂️ . I will see if i can activate it in our version...

Typical ODA 😆

@sebjf
Copy link
Contributor Author

sebjf commented Sep 20, 2024

@carmenfan

😆 Ok that worked!

So we can make everything grayscale. Is that an acceptable solution or would we rather preserve colours but make them darker?

Here is floor 2 of the revit house in grayscale:

image

If the latter, the exporter sample isn't too large - just a few source files - so we could embed it as source into bouncer (either into bouncer proper or as a separate module) and then modify it further to our needs.

@carmenfan
Copy link
Member

@sebjf just spot something when I was testing something unrelated:

It's trying to process auto calibration and failing when I'm passing in a PDF on this branch:

image

@sebjf
Copy link
Contributor Author

sebjf commented Sep 25, 2024

Hi @carmenfan ,

t's trying to process auto calibration and failing when I'm passing in a PDF on this branch:

This should be fixed now.

The branch is passing on Travis now too - turns out that the DGN test was loading the original SVGExporter from ODA, which despite not sharing the name of our version, and in the source not seeming to have anything else to identify it as the same library, was displacing ours which was loaded second...

The DGN importer has been updated accordingly, so once we have agreed a colour policy this will be ready for another look!

@carmenfan
Copy link
Member

Some reference pictures

min line width 0.01

image

image

min line width 0.08

image
image

min line width 0.08, RGB value halved

image
image

min line width 0.08, RGB value * 0.8

image
image

min line width 0.04, RGB value * 0.8

image
image

@zaneulhaq
Copy link

@carmenfan - Line width 0.08 with RGB 0.8 please

@carmenfan
Copy link
Member

@carmenfan - Line width 0.08 with RGB 0.8 please

tried the client files with this settings and they look fine (they have proper black lines so generally very visible anyway... lol) so let's go with this! @sebjf

@carmenfan
Copy link
Member

@sebjf on a side note - I still haven't managed to get an autocalibrated drawing in......... do you have something I can try?

@sebjf
Copy link
Contributor Author

sebjf commented Sep 27, 2024

Hi @carmenfan,

let's go with this!

Great, done!

I still haven't managed to get an autocalibrated drawing in

Can you say more about what you mean by not managed? (Is there a .io branch that uses the calibration I can test with now?)

Since when I wrote this the frontend wasn't using the calibration yet, I validated what it was writing to the database manually, by comparing vectors created with the calibration tool to what this branch outputs, using a function that I expect the frontend to implement the equivalent of (end of this comment). It was all correct at the time, but perhaps I missed something. Or do you mean it isn't writing to the database? If so that's definitely regressed!

@carmenfan
Copy link
Member

@sebjf

I validated what it was writing to the database manually,

This is what I meant, I haven't managed to get a file to import and write a calibration record. It doesn't do it for the customer files, and it didn't do it for the Revit file that exported with shared coordinate (The one I mentioned here)

@carmenfan
Copy link
Member

@sebjf ignore me............. I'm being blind!
image

I think i might've looked at staging database when i imported into local.... :'(

@carmenfan
Copy link
Member

@sebjf can we change this to drawings.calibrations please?

Just checked the io code and this is where they're being written to on 3drepo.io

https://github.com/3drepo/3drepo.io/blob/staging/backend/src/v5/models/calibrations.js#L20

@carmenfan
Copy link
Member

@sebjf this looks fine but the travis build failed 🙈

Copy link
Member

@carmenfan carmenfan left a comment

Choose a reason for hiding this comment

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

Travis failed

@sebjf
Copy link
Contributor Author

sebjf commented Oct 2, 2024

@carmenfan forgot to update the unit test for the new 0.8 colour policy 🤦‍♂️, it should pass now!

Copy link
Member

@carmenfan carmenfan left a comment

Choose a reason for hiding this comment

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

Verified with 3drepo/3drepo.io#5193 that auto calibration is working!

Can merge 🥳

@carmenfan carmenfan merged commit a86c0a5 into staging Oct 9, 2024
7 checks passed
@carmenfan carmenfan deleted the ISSUE_695 branch October 9, 2024 16:45
@carmenfan carmenfan removed their assignment Oct 15, 2024
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.

4 participants