Fix WMS shift and margins - #198
Conversation
[Maven Build Status]📑 Commit: 📦 Download artifact: Generator.jar |
ea1b379 to
d58c8b3
Compare
|
There are still some tests to perform on matrix/vector alignment (not sure of +/- 0.5 adjustments). |
| private Rounding() {} | ||
|
|
||
| /** | ||
| * Returns the closest double value that is divsible by {@code gap}. |
There was a problem hiding this comment.
| * Returns the closest double value that is divsible by {@code gap}. | |
| * Returns the closest double value that is divisible by {@code gap}. |
(Tu l'as fait partout, probablement un copié collé)
| * Returns the closest double value divsible by gap with some gaps added. | ||
| * | ||
| * @param number Number to round | ||
| * @param gap Rounding gap | ||
| * @param offset number of gaps to add to result | ||
| * @return closest number to {@code number} divsible by {@code gap} with {@code offset} gaps added |
There was a problem hiding this comment.
(C'est valable pour toutes ces javadocs)
Si je dis pas de bêtise, tu peux utiliser la syntaxe "courte" {@return ...} même quand il y a d'autres choses derrière. Ça évite de se répéter dans le @return
| * Returns the closest double value divsible by gap with some gaps added. | |
| * | |
| * @param number Number to round | |
| * @param gap Rounding gap | |
| * @param offset number of gaps to add to result | |
| * @return closest number to {@code number} divsible by {@code gap} with {@code offset} gaps added | |
| * {@return closest number to {@code number} divsible by {@code gap} with {@code offset} gaps added} | |
| * | |
| * @param number Number to round | |
| * @param gap Rounding gap | |
| * @param offset number of gaps to add to result |
Ceci dit, il faut tester comment ça rend d'avoir des {@code ...} à l'intérieur. Si ça lui pose problème, tant pis on laisse comme ça, sinon ça vaut le coup de factoriser
| } | ||
|
|
||
| /** | ||
| * Returns the largest inferior double value that is divsible by {@code gap}. |
There was a problem hiding this comment.
(Cf. ma remarque juste au dessus)
D'autant qu'en plus tu as essayé de ne pas écrire la même chose (je comprends, j'aurais probablement voulu faire pareil) mais ça apporte un résultat un peu confus au final.
Typiquement, je trouve "largest inferior double value" bien moins clair que "largest number less than or equals to number" qui ne laisse aucune ambiguïté
| assertValue(2, heightmap, 0, -1); // 0 | ||
| assertValue(4, heightmap, 1, -1); // 0 | ||
| assertValue(0, heightmap, -1, 0); | ||
| assertValue(4, heightmap, 0, 0); | ||
| assertValue(5, heightmap, 1, 0); | ||
| assertValue(8, heightmap, 0, 0); // 0 | ||
| assertValue(10, heightmap, 1, 0); // 6 |
There was a problem hiding this comment.
C'est quoi ces commentaires ? J'ai du mal à les comprendre
| */ | ||
| public class PopulateHeightmapTask extends ModelTask<FloatMatrixModel> { | ||
| private final WritableHeightmapSpec heightmapSpec; | ||
| private final double verticalScale; |
There was a problem hiding this comment.
La gestion du vertical scale ne devrait-elle pas rester dans la PR #155 ? Vu que ça serait le seul endroit ici où ça serait implémenté sinon, et cette PR devrait se contenter du correctif du décalage constaté sur le WMS ?
There was a problem hiding this comment.
Oui, actuellement elle dort, mais je besoin je peux rebase pour la merge
| double pixelSize = 1d; | ||
|
|
||
| // This is the WMS bbox expressed in map coordinates. | ||
| // It is used bellow to deduce matrix offset and cell size. |
There was a problem hiding this comment.
J'aime beaucoup la définition de "bellow" d'après Google :
Le mot "bellow" signifie crier fort ou désigne un animal qui mugit, mais il s'agit souvent d'une faute de frappe pour le mot "below" (en bas) ou d'une référence au célèbre écrivain Saul Bellow.
|
|
||
| // This is the WMS bbox expressed in map coordinates. | ||
| // It is used bellow to deduce matrix offset and cell size. | ||
| // WMS mattrix is alligned in the same way in all tiles (use of floor/ceil). |
There was a problem hiding this comment.
"mattrix is alligned bellow" 😄
| // WMS mattrix is alligned in the same way in all tiles (use of floor/ceil). | |
| // WMS matrix is aligned in the same way in all tiles (use of floor/ceil). |
| // WMS mattrix is alligned in the same way in all tiles (use of floor/ceil). | ||
| // This prevents glitches between tiles. | ||
|
|
||
| // We need margin for interpolation (-1/1 expressed in pixelSize) |
There was a problem hiding this comment.
| // We need margin for interpolation (-1/1 expressed in pixelSize) | |
| // We need margin for interpolation (-1/+1 expressed in pixelSize) |
| new Coordinate(bbox.minX(), bbox.minY()), | ||
| new Coordinate(bbox.maxX(), bbox.minY()), | ||
| new Coordinate(bbox.maxX(), bbox.maxY()), | ||
| new Coordinate(bbox.minX(), bbox.maxY()), | ||
| new Coordinate(bbox.minX(), bbox.minY()) |
There was a problem hiding this comment.
Initialement, il y avait +1 sur le max pour inclure le dernier voxel, mais c'était ça qui cachait le problème d'alignement (i.e. qui aurait dû être -0.5 / +0.5, mis à l'échelle) ?
Donc maintenant que c'est correctement aligné dans la requête, ça ne va pas causer de problème de ne plus faire ce +1 ici sur le max ?
| int width = (int) Math.round(envelope.getMaxX() - envelope.getMinX()); | ||
| int height = (int) Math.round(envelope.getMaxY() - envelope.getMinY()); | ||
|
|
||
| double pixelSize = 1d; |
There was a problem hiding this comment.
C'est du détail mais le d ici ne sert à rien puisque le type est déjà explicite double
| double pixelSize = 1d; | |
| double pixelSize = 1; |
|
Vérifier l'envelope Vecteur à laquelle il manque peut être quelque chose. |
3 participants
Changes
Fix a bug in WMS processing when angle was not 0/90/180/270. In corners
Technical changes
TODOs
(later) : Makes processor tell provider which margin it needs for interpolation (hardcoded for now)
Self-checks
The code has unit tests associatedThe code has Javadoc Comments associatedComplex / Unexpected code is explained / justified with a small commentRelevant documentation inside the/docsfolder has been updatedAll examples inexamples/work the same (or have been adapted if subject to changes in this PR)The texts have been proofread (documentation, error messages, logs, comments...)