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

Few changes #49

Merged
merged 5 commits into from
May 22, 2017
Merged

Few changes #49

merged 5 commits into from
May 22, 2017

Conversation

untereiner
Copy link
Collaborator

No description provided.

lionel untereiner added 3 commits May 19, 2017 11:38
one can rename CMakeOptions.txt.sample file to CMakeOptions.txt and
modify the path to cgogn.
This personalized file must not be commited.
@@ -385,6 +396,8 @@ void View::preDraw()

void View::draw()
{
glClearColor(background_color_.redF(), background_color_.greenF(), background_color_.blueF(), background_color_.alphaF());
Copy link
Member

Choose a reason for hiding this comment

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

Plutôt que d'appeler glClearColor dans chaque draw, est-ce qu'on ne peut pas l'appeler uniquement dans la méthode color_selected c'est-à-dire seulement quand la couleur du background change ? (il faudra peut-être faire un this->makeCurrent() avant).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@pierrekraemer
Copy link
Member

pierrekraemer commented May 19, 2017

Pour l'histoire du CGoGN path, je ne suis pas sûr qu'il soit plus intuitif d'avoir à éditer un fichier CMakeOptions plutôt que d'avoir à changer un champ dans la config cmake (par exemple via ccmake ou dans l'interface de QtCreator).
En tout cas, ça n'est pas moins "hard coded" dans un fichier CMakeOptions à modifier selon sa config personnelle que dans une variable "CACHE" déclarée dans le CMakeLists également à modifier selon sa config.

@untereiner
Copy link
Collaborator Author

sauf que le fichier CMakeOptions n'est jamais dans le système de gestion de version. A minima, je préférerai que l'on ne mette rien dans cette variable cache.

@pierrekraemer
Copy link
Member

De mon point de vue, les valeurs que l'on donne aux variables "cache" dans un CMakeLists tiennent le même rôle d'exemple de configuration à faire par l'utilisateur.

Dans le cas de la variable "cache", l'action à effectuer est simplement de saisir une bonne valeur via ccmake ou QTCreator.
Dans le cas d'un CMakeOptions, il faut créer le fichier adéquat (par exemple en copiant le fichier sample fourni), puis saisir les bonnes valeurs pour les variables.

Dans tous les cas, l'utilisateur doit saisir les bonnes valeurs pour les différentes variables. Dans tous les cas, on fournit un exemple de valeur pour chaque variable.

Le seul intérêt que je verrai à un fichier séparé est de regrouper en un seul lieu les variables pour lesquelles on est (quasi) sûr que l'utilisateur devra modifier l'exemple fourni (contrairement à d'autres variables plus simples et moins dépendantes de la configuration locale).

@untereiner
Copy link
Collaborator Author

mais également que ces modifications ne doivent pas être filtré du système de gestion de version. Mais ok, je garderai ca chez moi.

@pierrekraemer
Copy link
Member

Quand on configure une variable avec ccmake ou dans QTCreator, cela ne modifie en rien le CMakeLists qui est dans le dépôt (c'est stocké dans le fichier CMakeCache).

@untereiner
Copy link
Collaborator Author

je n'utilise ni qtcreator ni ccmake pour configurer cette variable

@pierrekraemer
Copy link
Member

Tu utilises quoi alors pour le faire sans toucher au CMakeLists ?

@untereiner
Copy link
Collaborator Author

j'édite mon fichier de config

@pierrekraemer
Copy link
Member

Oui mais avant de faire avec un fichier CMakeOptions séparé, tu faisais comment ?

@untereiner
Copy link
Collaborator Author

je faisais pas autrement, j'ai fais comme ca dans mon coin depuis le début

@pierrekraemer
Copy link
Member

Bon pour merge ?

@pierrekraemer pierrekraemer merged commit 5923a09 into cgogn:develop May 22, 2017
@untereiner untereiner deleted the few_changes branch May 23, 2017 06:54
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.

2 participants