Skip to content

GSoC 2018: 2D Boolean set Operations demo#3306

Closed
apurvakbh wants to merge 13 commits intoCGAL:masterfrom
apurvakbh:master
Closed

GSoC 2018: 2D Boolean set Operations demo#3306
apurvakbh wants to merge 13 commits intoCGAL:masterfrom
apurvakbh:master

Conversation

@apurvakbh
Copy link
Copy Markdown

@apurvakbh apurvakbh commented Sep 1, 2018

Please add this branch to the master branch.

The brach contains 2D Boolean set Operations demo. It was done as a part of the gsoc 2018 project. It was done under mentor-ship of Efi Fogel and Andreas Fabri.

TODO:

  • filter branch to reduce history size (@sloriot)
  • handle degenerate polygons

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Sep 3, 2018

I have many warnings when using -Wall and -Wextra, you should fix them to pass the testsuite.

@lrineau lrineau added this to the 4.14-beta milestone Oct 1, 2018
@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 1, 2018

Andreas, Efi, please comment/review this pull-request.

Cc: @afabri @efifogel

Copy link
Copy Markdown
Member

@efifogel efifogel left a comment

Choose a reason for hiding this comment

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

This program demonstrates the 2D regularized Boolean operations. It's the 1st release of such a demo program based on Qt5. Currently (excluding this demo) the 2D regularized operations are not demonstrated by any demo program, so it's a good intensive to release it. It lacks several features, but it's in a satisfactory shape. I do recommend the release.

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 8, 2018

This branch weights 2724kB. @sloriot, would you mind have a look if that is normal, or can be fixed?

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 10, 2018

There are a lot of warnings or compilation errors, in the testsuite:
https://cgal.geometryfactory.com/CGAL/testsuite/CGAL-4.14-Ic-14/Boolean_set_operations_2_Demo/TestReport_gimeno_CentOS6.gz

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Oct 10, 2018 via email

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 10, 2018

I do not see any push. What have you modified?

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Oct 10, 2018 via email

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 10, 2018

The push should be done in the master branch of the repo git@github.com:apurva786/cgal.git, that is the branch of this PR. If you do not have the rights for that, then either @apurva786 will have to do it, or we will replace this PR by another one you create.

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Oct 10, 2018 via email

@apurvakbh
Copy link
Copy Markdown
Author

This branch weights 2724kB. @sloriot, would you mind have a look if that is normal, or can be fixed?

The branch's size is big because it contains some images(in the resources folder). All of them are not used currently, but I have kept them in case they might be useful for future developers.

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Oct 11, 2018

@apurva786 commented on Oct 11, 2018, 6:28 AM GMT+2:

The branch's size is big because it contains some images(in the resources folder). All of them are not used currently, but I have kept them in case they might be useful for future developers.

That is true. And it is also big because you committed, then removed, binary files:

  • 6088a04 - 2D Boolean operations demo (6 weeks ago)
   .../Boolean_set_operations_2                 |  Bin 39101624 -> 0 bytes
   .../Boolean_set_operations_2.tar.gz          |  Bin 6316570 -> 0 bytes
  • 965b800 - 2D Boolean operations demo (6 weeks ago)
   .../Boolean_set_operations_2                      |     Bin 0 -> 39101624 bytes
   .../Boolean_set_operations_2.tar.gz               |     Bin 0 -> 6316570 bytes

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Oct 13, 2018 via email

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Oct 31, 2018

There are still many warnings.
See test results.

@sloriot sloriot added the TODO label Nov 5, 2018
@sloriot
Copy link
Copy Markdown
Member

sloriot commented Nov 5, 2018

@apurva786 could you please add me and @maxGimeno as collaborators?

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Nov 5, 2018

Is this the expected windows? I don't see any of my input polygon in the list.
screenshot from 2018-11-05 16-55-12

Also if I draw an invalid polygon, the demo simply crashes. We should add a test that consider a polygon only if it is valid and otherwise display an error message (a bad click can happen to anyone and we don't want the whole session to be gone for a bad click).

@apurvakbh
Copy link
Copy Markdown
Author

I have added @maxGimeno as a collaborator.
The screenshot attached in the above comment is the expected window.
After reading the comment, again I did some testing and indeed the demo crashes when the user draws a polygon which intersects with itself and it crashes without a warning. I am trying to figure out a way how to prevent it from crashing. If there are any other cases where it happens please let me know.

@maxGimeno can you please elaborate the statement "I don't see any of my input polygon in the list."

I would like to mention the console, Infos and the buttons under Geometric objects are disabled, but the radio buttons and checkbox are working.

The demo is expected to work like as shown in this video link

Regarding how to use the demo
I have made a documentation about how to use the demo. It is in help folder with the name index.html.

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Nov 6, 2018

Thanks @apurva786 , I was able to push.
About the polygon list, I thought that like in the polyhedron demo the polygons were added to a list but actually it is not the case, we somehow have a limited set of colors. What's the point of having the icons + the filter at the top of the "Geometric Objects" widget?

@apurvakbh
Copy link
Copy Markdown
Author

apurvakbh commented Nov 6, 2018

@sloriot The plus('+') sign is meant to be used to load a polygon in the demo from the stored polygons using file reading. It can be removed, and add the same function in the File drop down.

The minus('-') sign is not needed at all. It can be removed.

The third sign(which looks like a file) it can be used to duplicate the currently selected polygon.

The up and down keys are not useful now, but I have kept them in case it might be useful when the demo is enhanced to 3-D.

The same goes for the tab with "Quick filter" written in it. Might be useful for 3-D.

We can remove those UI components for now. It won't take much time for the future developers to add them again.

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Nov 6, 2018 via email

@apurvakbh
Copy link
Copy Markdown
Author

Thank you @efifogel I will try to use these functions.

@maxGimeno
Copy link
Copy Markdown
Contributor

Hello, what is the status of this branch ? @sloriot it is written that you have some filtering to do, but also there is something to be done about degenerated polygons.

@lrineau
Copy link
Copy Markdown
Member

lrineau commented Jan 24, 2019

@efifogel @apurva786 No change to the PR since November 2018. This PR will probably not make it into CGAL-4.14.

@maxGimeno maxGimeno modified the milestones: 4.14-beta, 4.15-beta Jan 25, 2019
@lrineau lrineau added rm: not for next release Indicate to the release team that a PR should not be merged before the next release branch is forked and removed rm: not for next release Indicate to the release team that a PR should not be merged before the next release branch is forked labels Jan 28, 2019
@lrineau lrineau removed the rm: not for next release Indicate to the release team that a PR should not be merged before the next release branch is forked label Apr 1, 2019
@maxGimeno
Copy link
Copy Markdown
Contributor

Still no change, I move it to Attic/Trash

@maxGimeno maxGimeno modified the milestones: 5.0-beta, Trash / Attic Jun 17, 2019
@sloriot
Copy link
Copy Markdown
Member

sloriot commented Jan 3, 2022

@efifogel what is the status of this pull request?

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Jan 4, 2022 via email

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Dec 18, 2023

@efifogel If I got it right, we can close this PR?

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Dec 19, 2023 via email

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Dec 19, 2023

I don't want to close it but my understanding is that this work is obsolete and another branch does exist (but maybe I misunderstood)

@efifogel
Copy link
Copy Markdown
Member

efifogel commented Dec 19, 2023 via email

@sloriot sloriot closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants