Add MapRender for quads #316

kvanhoey · 2017-08-11T06:33:12Z

Adding a MapRender for quadrilateral rendering, allowing to write shaders to do quad interpolation.
Tested on a personal side-application with quad interpolation enabled.

pierrekraemer · 2017-09-08T09:34:45Z

@kvanhoey la classe MapQuadRender pourrait facilement être intégrée à la classe MapRender en ajoutant simplement "QUAD" dans l'enum DrawingType.
Ca serait plus simple, non ?

kvanhoey · 2017-09-12T14:30:42Z

C'est fait et pushé.

pierrekraemer · 2017-09-12T14:44:26Z

cgogn/rendering/map_render.h

@@ -261,6 +282,8 @@ class CGOGN_RENDERING_API MapRender
 					init_triangles(m, mask, table_indices);
 				indices_tri_.clear();
 				break;
+            case QUAD:
+                init_quad(m, mask, table_indices) ;


Il manque un break ici.

Corrigé. Désolé.

pierrekraemer · 2017-09-12T14:47:02Z

Petite remarque globale d'ordre esthétique : on utilise partout des tabs pour indenter le code. Tes ajouts sont indentés avec des espaces. Est-ce que tu pourrais modifier ça pour mettre des tabs ?

untereiner · 2017-09-12T15:13:08Z

J'ai l'impression que d'un point de vue topo les objets square et parallélogramme sont des objets similaires. Ce qui change c'est la manière de les plonger. D'ailleurs, je me dis que le mot square est mal choisi. Cet objet construit une grille de faces à 4 côtés.

pierrekraemer · 2017-09-12T15:26:23Z

Si j'ai bien compris, il me semble que le problème dans ce cas est que la topo et le plongement sont construits conjointement. On ne peut pas, comme dans les autres exemples de tiling, construire d'une part la topo (dont la nature peut être tri, quad, ..) puis plonger la grille obtenue. On ne peut donc pas utiliser l'objet SquareGrid (qui pourrait effectivement être renommée en QuadGrid) et lui ajouter un nouvelle méthode embed_into_xxx. Il faut nécessairement quelque chose de séparé et propre au problème.

kvanhoey · 2017-09-12T15:54:35Z

Arf...
Désolé pour le break. C'est corrigé.
Pour l'indentation, je vais voir pour changer ça en effet, je ne m'en suis pas rendu compte.
Et pour le parallélogramme, je ne voulais pas le commiter à la base, mais ma mauvaise habitude de faire du commit -a -m m'a eu.

Je suis d'accord avec Lionel que Square est mal choisi.

Et pour le parallélogramme, ce qui change c'est que pour mon appli je dois absolument construire la topo et le plongement conjointement. C'est pas séparable (on pourra en discuter si tu veux), car je veux construire une topo qui couvre une certaine surface (un rectangle ; le critère d'arrêt est sur le plongement).
Je confirme donc totalement ce que Pierre dit.

En l'occurence, c'est peut-être pas opportun de pusher ça dans CGoGN ...

kvanhoey · 2017-09-12T15:57:36Z

Dans QtCreator -> Options -> Text Editor -> Behavior -> Tab Policy
je suis en "Tab Only" (par opposition à "Spaces only" ou "mixed"). Du coup, je ne vois pas bien quoi changer, à part l'éditeur utilisé ...

untereiner · 2017-09-12T15:57:58Z

Utilise le style QtCreator à la racine de cgogn ;)

untereiner · 2017-09-12T16:01:15Z

Sinon c'était pas une question d'opportun ou pas. Ce que j'ai compris de l'algo m'a fait pensé que c'était factorisable. Au départ est-ce que tu connais la taille de la région à couvrir et le niveau de discrétisation (combien de sommets/faces en X et en Y) de cette région ?

kvanhoey · 2017-09-12T16:04:27Z

J'utilise bien le IGGstyle ... malheureusement, ça marche pas.

kvanhoey · 2017-09-12T16:08:40Z

Et oui, je connais la taille à couvrir et le parallelogramme (quelconque) que je veux (sa forme et donc taille géométrique)... c'est pas exclus que la topo soit pré-calculable sans plonger, mais ça reviendrait au même, car il faudrait calculer au fur et à mesure de la construction combien sera couvert déjà ...

Un screenshot de ce que je veux faire: couvrir une texture de parallelogrammes quelconques (et je vais devoir générer des millieurs de grilles avec des parallelogrammes différents):

Tout en ayant au moins un parallélogramme complet de marge entre le bord de la texture et le bord du maillage.

kvanhoey · 2017-09-12T16:13:36Z

C'est bon, les tabs c'est fait et pushé !

etienneschmitt · 2017-09-13T09:28:05Z

cgogn/modeling/tiling/parallelogram_grid.cpp

+namespace modeling
+{
+
+//template class CGOGN_MODELING_API ParallelogramGrid<CMap2,Eigen::Vector2d>;


Est-ce qu'il y a une raison pour laquelle c'est commenté ?

Non. Ou du moins, je ne m'en souviens pas. En fait, je devrais vraiment exclure cette classe du pull request car cette classe n'est absolument pas finalisée (c'était un effet de bord de mon pull request pour le MapRender).
C'est faisable facilement ?

Je pense que le plus simple est de faire un commit dans lequel tu enlèves les parties non-finalisées. Sinon ça peut très bien rester en l'état le temps que tu travailles dessus, faut voir avec Pierre.

Ouai mais j'aimerais quand même avoir le commit et le push sur mon github à des fins de backup ... mais pas dans le pull request pour ne pas polluer CGoGN.
@pierrekraemer, un avis ?

Tu peux créer une nouvelle branche de backup à partir de celle que tu utilises pour la PR. Puis tu continues tes modifs dans l'ancienne branche.

etienneschmitt · 2017-09-13T09:30:07Z

cgogn/modeling/tiling/parallelogram_grid.h

+	bool is_valid() const
+	{
+		bool res = Ti_[0] > 0 && Tj_[0] >= 0 && Tj_[1] > Ti_[1] ;
+		cgogn_assert((acos(-Tj_[1] / Tj_.norm()) > acos(-Ti_[1] / Ti_.norm())) == res) ;


Ici je pense qu'il faudrait éviter le cgogn_assert dans la méthode is_valid ? vu que plus haut tu fais déjà un cgogn_assert(is_valid()).

Au passage: c'est quoi la différence entre les assert et les ensure ?

Aucune diff en dehors du nom.

etienneschmitt · 2017-09-13T09:32:34Z

cgogn/modeling/tiling/parallelogram_grid.h

+	{
+		bool res = Ti_[0] > 0 && Tj_[0] >= 0 && Tj_[1] > Ti_[1] ;
+		cgogn_assert((acos(-Tj_[1] / Tj_.norm()) > acos(-Ti_[1] / Ti_.norm())) == res) ;
+		return res && p_.norm() > 1e-10 && Ti_.norm() > 1e-10 && Tj_.norm() > 1e-10 ;


On a des fonctions almost_equal_absolute et almost_equal_relative qui pourraient être utilisées.

pierrekraemer · 2017-09-13T14:30:24Z

@kvanhoey Ce n'est pas très embêtant que ces classes dédiées aux parallélogrammes intègrent la lib. Elles n'interfèrent pas avec le reste de toute manière. En revanche, on peut laisser la PR ouverte tant que ça n'est pas terminé si tu veux.
Si tu préfères ne pas les intégrer, alors tu peux faire comme proposé par @etienneschmitt : à savoir, tu crées une nouvelle branche "parallelogram" à partir de ton commit courant, tu reviens sur la branche "kvanhoey", source de cette PR, et tu ajoutes un commit qui supprime ces fichiers et tu pushes, modifiant ainsi cette PR. Tu peux ensuite retourner sur la branche "parallelogram" et continuer à travailler comme si de rien n'était. En revanche, je pense que des problèmes apparaitront quand tu voudras intégrer des mises-à-jour de CGoGN vers cette branche..

sylvainthery · 2017-09-14T07:16:44Z

Pour info comment ne pas polluer une PR en continue avec un WIP:

checkout -b work
bla bla commit push bla bla commit push PR work ->cgogn:develop
Ahh je veux continuer !!
checkout -b work_continue (depuis work)
bla bla commit push bla bla commit push
checkout work + merge work_continue + push

Il faut faire des branches, c'est l'un des points forts de git

Sinon tu peux mettre dans l'index (et donc commiter) des morceaux de fichier avec git add -i

Mais une fois que c'est commité et pushé c'est trop tard ( sauf remove + push)

kvanhoey · 2017-09-14T10:27:54Z

Merci pour vos suggestions, j'ai bien noté la recommandation "faites des branches!" ;).

Je propose de faire au plus simple pour l'instant: laisser la PR ouverte tant que ça n'a pas convergé.
Si quelqu'un a besoin de rendu QUAD, vous commentez ici et on ré-évalue ça.

kvanhoey · 2017-10-11T10:42:35Z

Hmmm ... des avis sur les "checks".
@pierrekraemer : j'ai rien changé depuis vendredi ...

pierrekraemer · 2017-10-11T12:30:07Z

Ah oui pardon les checks ne sont pas configurés pour le moment et échouent systématiquement.. Il ne faut pas en tenir compte. Désolé..

kvanhoey · 2017-10-27T14:00:17Z

Je viens d'ajouter un fichier "geometry/algos/convexity.h" qui vérifie si une face planaire est convexe. Il y a donc deux fonctions: isPlanar et isConvex. J'ai testé le isConvex dans le cas 2D (avec Vec2 et Vec3), pas au-delà. Je pense que ça peut être utile à intégrer, mais c'est à votre discretion bien sûr ;).

pierrekraemer · 2017-10-27T16:39:08Z

Pour info, la convention de nommage pour les fonctions et méthodes dans CGoGN 2 est le snake_case..

kvanhoey · 2017-10-27T16:55:56Z

Ok, je fais une passe :)

…

On 27/10/2017 18:39, Pierre Kraemer wrote: Pour info, la convention de nommage pour les fonctions et méthodes dans CGoGN 2 est le snake_case.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#316 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVDKQgIdXh5n8L5hvRwbiYhPMwFxrnQkks5swgcsgaJpZM4O0Sn7>.

Kenneth Vanhoey added 10 commits June 28, 2017 11:18

comment typo

631d4d9

Merge branch 'develop' of github.com:cgogn/CGoGN_2 into kvanhoey

19580b4

Merge branch 'develop' of github.com:cgogn/CGoGN_1 into kvanhoey

c63c9f6

Add map_quad_render

1504c97

Cleanup MapQuadRender

9f9efc6

add ParallelogramTiling Topo. Ongoing ...

53c7b8e

paralellogram_grid

100adff

cleanup for parallelogram grid

7dca195

code refactoring on parallelogram_grid

4bee075

code refactoring on parallelogram_grid

7e6a4e7

Kenneth Vanhoey added 3 commits September 12, 2017 16:27

integrate MapQuadRender into MapRender with an enum QUAD

4abd9c5

Merge branch 'develop' of github.com:cgogn/CGoGN_2 into kvanhoey

f5e2681

update CMakeList accordingly

ff110a1

pierrekraemer reviewed Sep 12, 2017

View reviewed changes

add missing break; instruction

ad23554

change spaces to tabs

a099b06

etienneschmitt reviewed Sep 13, 2017

View reviewed changes

Kenneth Vanhoey added 2 commits September 13, 2017 13:18

sampling parallelograms

e3d0cc5

sample parallelograms

f50c66f

Kenneth Vanhoey and others added 9 commits September 18, 2017 18:14

Sample parallelograms updated

c12be0e

Sample parallelograms updated

c2dcb16

bug fixing

b0726f5

double templates for parallelogram_grid

4921587

replace assert by warning

9717168

compil fix

1eb98f4

Merge branch 'develop' of github.com:cgogn/CGoGN_2 into kvanhoey

c24b111

tmp commit

869cc70

fix conflict

cca187d

adding planarity and convexity tests for faces

71ffd9c

add parallelogram::isApprox

cbd1ec9

kvanhoey and others added 4 commits October 27, 2017 19:02

correct snake_case

082bba6

correct snake_case

0cad941

uint -> uint32 for windows compatibility

bce8843

uint -> uint32 for windows compatibility

af487e9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MapRender for quads #316

Add MapRender for quads #316

kvanhoey commented Aug 11, 2017

pierrekraemer commented Sep 8, 2017

kvanhoey commented Sep 12, 2017

pierrekraemer Sep 12, 2017

kvanhoey Sep 12, 2017

pierrekraemer commented Sep 12, 2017

untereiner commented Sep 12, 2017

pierrekraemer commented Sep 12, 2017

kvanhoey commented Sep 12, 2017

kvanhoey commented Sep 12, 2017 •

edited

Loading

untereiner commented Sep 12, 2017

untereiner commented Sep 12, 2017

kvanhoey commented Sep 12, 2017

kvanhoey commented Sep 12, 2017 •

edited

Loading

kvanhoey commented Sep 12, 2017

etienneschmitt Sep 13, 2017

kvanhoey Sep 13, 2017

etienneschmitt Sep 13, 2017

kvanhoey Sep 13, 2017

etienneschmitt Sep 13, 2017

etienneschmitt Sep 13, 2017

kvanhoey Sep 13, 2017

kvanhoey Sep 13, 2017

etienneschmitt Sep 13, 2017

etienneschmitt Sep 13, 2017

kvanhoey Sep 13, 2017

pierrekraemer commented Sep 13, 2017

sylvainthery commented Sep 14, 2017

kvanhoey commented Sep 14, 2017

kvanhoey commented Oct 11, 2017

pierrekraemer commented Oct 11, 2017

kvanhoey commented Oct 27, 2017

pierrekraemer commented Oct 27, 2017

kvanhoey commented Oct 27, 2017 via email

Add MapRender for quads #316

Are you sure you want to change the base?

Add MapRender for quads #316

Conversation

kvanhoey commented Aug 11, 2017

pierrekraemer commented Sep 8, 2017

kvanhoey commented Sep 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierrekraemer commented Sep 12, 2017

untereiner commented Sep 12, 2017

pierrekraemer commented Sep 12, 2017

kvanhoey commented Sep 12, 2017

kvanhoey commented Sep 12, 2017 • edited Loading

untereiner commented Sep 12, 2017

untereiner commented Sep 12, 2017

kvanhoey commented Sep 12, 2017

kvanhoey commented Sep 12, 2017 • edited Loading

kvanhoey commented Sep 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierrekraemer commented Sep 13, 2017

sylvainthery commented Sep 14, 2017

kvanhoey commented Sep 14, 2017

kvanhoey commented Oct 11, 2017

pierrekraemer commented Oct 11, 2017

kvanhoey commented Oct 27, 2017

pierrekraemer commented Oct 27, 2017

kvanhoey commented Oct 27, 2017 via email

kvanhoey commented Sep 12, 2017 •

edited

Loading

kvanhoey commented Sep 12, 2017 •

edited

Loading