Issue 587 projected ticks #1144
Merged
+338
−262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
decided to simply add sub-point to line objects when projected so that it follows curves nicely box1 works, need to make sure ticks do the same as well
but polar still not drawing ticks correctly
in fitToViewport for polar poj
no more many pass in the long loop for proj
| self._renderers[(None,None,None)]=ren | ||
| else: | ||
| ren = self._renderers[(None,None,None)] | ||
| #if not (None,None,None) in self._renderers.keys(): |
There was a problem hiding this comment.
Is there a reason to save this?
|
Re "I didn't run ctest", please run ctest locally before making a pull request. It makes things a lot easier and faster if you test and fix issues before review. Also, please use a tool like |
|
@dlonie I didn't run ctest because I was out sick and I didn't have time. I knew it would fail and I was merely mentioning it to let you guys know it needs a new baseline which I didn't have time to generate last week (i.e. run 'ctest' to generate the new pngs) |
thanks to @dlonie for pointing us in the right direction fix CDAT#1143 priority was set on Renderer after the fitToViewport which was causing Renderers' layer to be reset to possibily too high/low priorities. now different renderers are created if they have different priorities
|
@dlonie ok it "works" the first time I build. Subsequent calls to the script do not draw the isofill anymore... |
having up to 200 calls to plot per priority seems to be enough
I still randomly get no data displayed, less frequently but still happens...
all non-projected baselines seems to be good again
|
I'm back in today -- where is this branch at? |
|
nvm -- I see the email now. |
|
@doutriaux1 I've updated #1143 with some more info. I can't do much more at this point, as I'm not sure what this code is trying to do, but this seems to be the point of failure. |
|
@aashish24 @dlonie I replied there. I guess VTK needs to figure out why the geo transfromreturns different values with the same inputs.... It is super worrisome... Anyhow I have a (slower) work around for now. Will push it through soon, thanks for pin-pointing the error. |
|
Have you tried using numbers smaller than +/- 100,000,000,000,000,000,000? I have a feeling that may be giving the PROJ4 backend fits. |
I had a leftover setNumberOfPoints(1) and then using InsertNextPoint, hence I had one extra point with random memory bits in it also moved pts=vtk.vtkPoints() out of the nested loop
|
@dlonie turns out I had a left over pt.SetNumberOfPoints(1) a few lines above, but now i the new code I switch to use InsertNextPoint that left us with 1 extra points which was probably filled with memory garbage hence the randomness. |
|
@dlonie we are very very close... BUT no cigar.... @aashish24 please help as well... bad ctest: which when run as follow: gives out: |
|
use to backtrace somewhere in geotranform. Not quite sure what's going on. |
|
earlier traceback (before the prints and setting pts number of points |
|
hum... using InsertPoint(n,x,y,z) rather than SetPoint(n,x,y,z) seems to actually fix the issue... odd... |
it showed it baselines ratio needed to be set to none for ticks/box otherwise would be stretched in bg mode
|
@dlonie @aashish24 I think this is ready to to go now, please review |
|
SetPoint assumes that memory is already allocated. InsertPoint allocates the memory for you. It should be noted that InsertPoint is slow. |
|
Works here. |
|
@aashish24 that was my understanding doesn't setNumberOfPoints allocate the memory? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@williams13 @aashish24 @chaosphere2112 I think this should go in for the rlease. It's better than what's in master.
BUT: polar projection have ticks drawn behind data. @dlonie can't look at it until later, but it souldn't be a show stopper.
Also I didn't run ctst, but for sure ALL projection tests will fail because a new baseline is needed for projected plots. @chaosphere2112 or @aashish24 can generate the new baseline.