Skip to content

Improve clip path for images in -Qi#6523

Merged
PaulWessel merged 3 commits intomasterfrom
grdview-clip-image
Apr 5, 2022
Merged

Improve clip path for images in -Qi#6523
PaulWessel merged 3 commits intomasterfrom
grdview-clip-image

Conversation

@PaulWessel
Copy link
Member

The algorithm used for building the staircase clip path for the image in grdview -Qi was somewhat smart but not smart enough, leaving a path when there is no valid image to the far left or right. While the clipping worked fine, the fact that the path extends too far means ghostscript senses these excursions and hence the bounding box is too large.

This PR eliminates that problem. I also added more comments to explain the otherwise cryptic section. The plot below show an actual 3-D image clip path. The BB of the full projected image is 0/8.37/0/3. The heavy line was the previous version and the new version is the thin red line.

No tests were affected.

mask

THe algorithm used for building the staircase clip path for the image in grdview -Qi was somewhat smart but not smart enough, leaving a path when there is no valid image to the far left or right.  WHile clipping is fine, the fact that the path extends too far means ghostscript senses these and the bounding box is too large.

This PR eliminates that problem.  I also added more comments to explain the otherwise cryptic section.  THe plot below show an actual 3-D image clip path.  The heavy line was the previous version and the new version is the thin yellow line.

No tests were affected.
@PaulWessel PaulWessel added the bug Something isn't working label Apr 5, 2022
@PaulWessel PaulWessel added this to the 6.4.0 milestone Apr 5, 2022
@PaulWessel PaulWessel requested review from joa-quim and maxrjones April 5, 2022 17:15
@PaulWessel PaulWessel self-assigned this Apr 5, 2022
@maxrjones
Copy link
Member

There are a few failures that need updating. Do you want to try out the process? The steps are below.

  1. Copy updated .ps files to test/baseline/grdview

From the base of the repository, run:

  1. dvc diff to check the appropriate files were udpated
  2. dvc add test/baseline/grdview
  3. git add test/baseline/grdview.dvc
  4. Run git commit -m "Update baseline images for grdview"
  5. Run dvc push
  6. Run git push

@PaulWessel
Copy link
Member Author

I am not sure what those failures are. I did dvc pull and status to make sure, then ran all tests. I only have the usual gspline_5 an testsuite. Are you seeing anything else in any grdview scripts?

@maxrjones
Copy link
Member

I am not sure what those failures are. I did dvc pull and status to make sure, then ran all tests. I only have the usual gspline_5 an testsuite. Are you seeing anything else in any grdview scripts?

Weird. I just redid all the steps and still get these failures:

The following tests FAILED:
	569 - test/grdview/autointense.sh (Failed)
	572 - test/grdview/denver.sh (Failed)
	573 - test/grdview/grdview.sh (Failed)
	576 - test/grdview/texture.sh (Failed)

Here are the diffs:
texture_diff

grdview_diff

denver_diff

autointense_diff

@PaulWessel
Copy link
Member Author

Those vertical thin lines? That ought to be the improvements - i,e. those lines were there before but now are gone in the the new files?

@PaulWessel
Copy link
Member Author

Just making sure you git pulled after I first published the branch since I updated it (before making a PR though). That version did leave some vertical lines at the end but the current version do not.

@maxrjones
Copy link
Member

Just making sure you git pulled after I first published the branch since I updated it (before making a PR though). That version did leave some vertical lines at the end but the current version do not.

Yes, I just checked again with a git pull and reran the tests. Is your branch also up-to-date based on git status (i.e., no local changes not available in this PR)?

@PaulWessel
Copy link
Member Author

pwd
/Users/pwessel/UH/RESEARCH/CVSPROJECTS/GMTdev/gmt-dev

(base) pwessel@MacAttack-> git status
On branch grdview-clip-image
Your branch is up to date with 'origin/grdview-clip-image'.

nothing to commit, working tree clean

dvc status
Data and pipelines are up to date. 

@PaulWessel
Copy link
Member Author

Looks like the CI tests are successful (or are they only partial?)

@maxrjones
Copy link
Member

The CI tests are partial. I'm running the tests now on my linux machine to see what happens (but it's slow).

Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Tests pass on Linux locally for me. Let's see what the CI tests say about the macOS confusion.

@PaulWessel
Copy link
Member Author

SO should I merge? Or are we waiting for something else?

@maxrjones
Copy link
Member

SO should I merge? Or are we waiting for something else?

Yes, you can merge. The full tests only run on PRs that are merged or that uncommented

# pull_request:
.

@PaulWessel PaulWessel merged commit e1f692c into master Apr 5, 2022
@PaulWessel PaulWessel deleted the grdview-clip-image branch April 5, 2022 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants