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

Mod: Show regressions in shape colouring #13372

Merged
merged 1 commit into from Apr 9, 2024

Conversation

wwmayer
Copy link
Contributor

@wwmayer wwmayer commented Apr 9, 2024

This PR demonstrates the known regressions of the recently merged material branch:

  • Changing the transparency after setting color per face will reset them
  • The result of boolean operations or compound doesn't inherit the colour of its input objects
  • If colour is set per face to a boolean operaton object then saving and restoring the file causes weird rendering behaviour because material binding is set to PER_PART but only a single colour is defined
  • If a shape inside a part container has set colour per face then saving and restoring as STEP file causes weird rendering behaviour for the same reason
  • Shape binder or datum objects don't show the correct default shape colour

@github-actions github-actions bot added WB Part Design Related to the Part Design Workbench WB Part Related to the Part Workbench labels Apr 9, 2024
This PR demonstrates the known regressions of the recently merged material branch:
* Changing the transparency after setting color per face will reset them
* The result of boolean operations or compound doesn't inherit the colour of its input objects
* If colour is set per face to a boolean operaton object then saving and restoring the file causes weird rendering behaviour
  because material binding is set to PER_PART but only a single colour is defined
* If a shape inside a part container has set colour per face then saving and restoring as STEP file causes weird rendering
behaviour for the same reason
* Shape binder or datum objects don't show the correct default shape colour
@wwmayer
Copy link
Contributor Author

wwmayer commented Apr 9, 2024

As expected these tests fail:

FAILED: testMultiFuse (parttests.ColorPerFaceTest.ColorPerFaceTest.testMultiFuse)
FAILED: testMultiFuseSaveRestore (parttests.ColorPerFaceTest.ColorPerFaceTest.testMultiFuseSaveRestore)
FAILED: testTransparency (parttests.ColorPerFaceTest.ColorPerFaceTest.testTransparency)
FAILED: testDefaultColor (TestPartDesignGui.TestDatumPlane.testDefaultColor)
FAILED: testDefaultColor (TestPartDesignGui.TestShapeBinder.testDefaultColor)
FAILED: testDefaultColor (TestPartDesignGui.TestSubShapeBinder.testDefaultColor)
FAILED: testSaveLoadStepFile (TestImportGui.ExportImportTest)

The MacOS build doesn't fail because it doesn't run the GUI tests.

@wwmayer wwmayer merged commit 3a34fe0 into FreeCAD:main Apr 9, 2024
6 of 9 checks passed
@wwmayer wwmayer deleted the test_show_regression branch April 9, 2024 20:13
@FlachyJoe
Copy link
Contributor

I thought that tests are created to avoid regression, what is the point of merging known failing tests?

@wwmayer
Copy link
Contributor Author

wwmayer commented Apr 9, 2024

Are you aware of the recent regressions due to the merged material branch? These tests do only fail because the broken code IS already in main. So, it's exactly the other way around: the broken has already been merged and these tests just show in a testable way that the code is broken.

To confirm that they test the correct things I have used them with an older version before the material branch was merged and there they all pass

With #13332 a new PR is available that makes many tests pass now. That PR I have rebased and force-pushed to confirm that most the tests will pass. This way it's easier to see that it (almost) works.

@FlachyJoe
Copy link
Contributor

I understand but I think I'd keep the tests in my dev branch until the corrective was ready. With this merge all the new submitted PR fail so it's harder to know their quality.

@luzpaz luzpaz added the Materials Materials related label Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Materials Materials related WB Part Design Related to the Part Design Workbench WB Part Related to the Part Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants