trevorsandy / lpub3d

An LDraw™ editor for LEGO® style digital building instructions.
https://trevorsandy.github.io/lpub3d/
130 stars 19 forks source link

Multi-level inherited colour seems to have lost assembly count in v2.4.6 r322 #723

Closed d-j-m-0 closed 1 year ago

d-j-m-0 commented 1 year ago

Subject

The addition of inherited colour consideration to add multiple instances to the instructions seems to have impacted the counting of the sub-assemblies.

In version 2.3.4 r294 the sub-assembly count was correct. In version 2.4.6 r322, the assembly sub-count when there are multiple levels of passing the inherited colour appears to result in a loss of the correct count.

Release r294:

r294

Release r322:

r322

Environment

Steps to reproduce

The attached zip file contains a sample mpd. There are also two PDFs in the zip file generated from the mpd. One PDF is generated with v2.4.6 r294 and the other with v2.4.6 r322.

Try generating the PDF for the mpd and see the results.

Expected behaviour

The sub-assembly count in using v2.4.6 r322 should be the same as that when using v2.4.6 r294.

Actual behaviour

The count sometimes seems to be 1 and sometimes 0!

Workaround

None known.

Solution suggestion

I wonder if this has something to do with multiple levels of colour inheritance. When there is a single level of inheritance, the sub-assembly counts seem to be correct.

inherited_multi_level.zip

trevorsandy commented 1 year ago

Thank you for reporting this behaviour.

Indeed. I noticed this item after pushing r324. As I'm treating some discovered regressions, I've yet to push the correction.

The following lines should be:

if (!countByColour || argv[1] == LDRAW_MAIN_MATERIAL_COLOUR)

https://github.com/trevorsandy/lpub3d/blob/f161acba78c4a1045cfddb9183c244aaa1feff52/mainApp/metaitem.cpp#L350

https://github.com/trevorsandy/lpub3d/blob/f161acba78c4a1045cfddb9183c244aaa1feff52/mainApp/metaitem.cpp#L443

Behaviour with correction. inherited_multi-level

Cheers,

trevorsandy commented 1 year ago

This behaviour has been corrected.

Cheers,