Skip to content

Improve -T in grdimage/view for transparency#7326

Merged
PaulWessel merged 1 commit intomasterfrom
trans-improv
Mar 13, 2023
Merged

Improve -T in grdimage/view for transparency#7326
PaulWessel merged 1 commit intomasterfrom
trans-improv

Conversation

@PaulWessel
Copy link
Member

Hoping this helps with #7315. Perhaps @anbj can give it a spin.

@PaulWessel PaulWessel added the bug Something isn't working label Mar 12, 2023
@PaulWessel PaulWessel added this to the 6.5.0 milestone Mar 12, 2023
@PaulWessel PaulWessel requested a review from anbj March 12, 2023 17:06
@PaulWessel PaulWessel self-assigned this Mar 12, 2023
n = gmt_graticule_path (GMT, &xx, &yy, 1, true, G->x[col] - inc2[GMT_X], G->x[col] + inc2[GMT_X], G->y[row] - inc2[GMT_Y], G->y[row] + inc2[GMT_Y]);
S->data[GMT_X] = xx; S->data[GMT_Y] = yy; S->n_rows = n;
gmt_geo_polygons (GMT, S);
gmt_M_free (GMT, xx);
Copy link
Member

Choose a reason for hiding this comment

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

Can't those guys be freed outside the loop?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, unless you rewrite it. n can vary from a few to tens or more depending on projection and MAP_LINE_STEP. I did not notice that it was particularly slow?

Copy link
Contributor

@anbj anbj left a comment

Choose a reason for hiding this comment

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

I think this is good to go.

-T+oblack+s -t40, -T+oblack@20+s -t50 and -T+oblack@20+s from #7315 (comment) now all give expected results. Let me know if you want graphical evidence.

@PaulWessel PaulWessel merged commit 74246bf into master Mar 13, 2023
@PaulWessel PaulWessel deleted the trans-improv branch March 13, 2023 15:27
@joa-quim
Copy link
Member

Allocating and freeing inside a double loop can never be cheap. Couldn't gmt_graticule_path() receive static x,y arrays, sufficiently large, set the non-used elements to 0 for safety, and return the number of used elements?

@PaulWessel
Copy link
Member Author

Since you don't know the number needed, I would recommend something like this:

  1. Pass the same arrays in, no freeing inside loop
  2. Pass in a size_t *n_alloc argument, initially 0
  3. Inside we check before each alloc if n < *n_alloc, and only alloc if too small.
  4. Increase *n_alloc for each alloc (there are 4 sides)
  5. Free arrays outside the loop in the end

This means in practice that we do most allocations early on and then we just recycle. Or as you suggest, start with n_alloc = GMT_LEN256 or something and then perhaps very little allocations happen.

If you have the time to do this - which I think is pretty simple, knock yourself out!

@joa-quim
Copy link
Member

These two weeks are quite busy but I'll see.

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.

3 participants