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

d.rast.num: Align grid with/without -a preserving the aspect ratio #3505

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HuidaeCho
Copy link
Member

This PR addresses #3503 by using the same pattern in d.rast.arrow.

@HuidaeCho HuidaeCho self-assigned this Mar 15, 2024
@github-actions github-actions bot added C Related code is in C module display labels Mar 15, 2024
@HuidaeCho HuidaeCho linked an issue Mar 15, 2024 that may be closed by this pull request
@HuidaeCho HuidaeCho added bug Something isn't working GUI wxGUI related raster Related to raster data processing and removed C Related code is in C module labels Mar 15, 2024
@HuidaeCho
Copy link
Member Author

The second 0 in

D_setup2(0, 0, t, b, l, r);
means don't adjust the destination coordinate system to preserve the aspect ratio.

grass/lib/display/setup.c

Lines 97 to 114 in 827f988

/*!
\brief Sets source coordinate system
Sets the source coordinate system to its arguments, and if
the <b>fit</b> argument is non-zero, adjusts the destination coordinate
system to preserve the aspect ratio.
If <b>clear</b> is true, the frame is cleared (same as running
<i>d.erase</i>). Otherwise, it is not cleared.
\param clear non-zero code to clear the frame
\param fit non-zero code to adjust destination coordinate system
\param s_top
\param s_bottom
\param s_left
\param s_right
*/
void D_setup2(int clear, int fit, double st, double sb, double sl, double sr)

However, setting that argument to 0 only works for the -a flag because the number of cells within the display extent counts actual raster cells, not what's shown with constrained resolution (for no -a flag). This is why d.rast.arrow works fine in d.mon wx0 with/without -a.

@echoix
Copy link
Member

echoix commented Mar 25, 2024

Who's knowledgeable to review this, and review this in relation to the other half dozen PRs related to the same problem/fixes? I personally didn't follow after the approach changed in one of the d.* PRs, and I'm lost with the discussions scattered.

@wenzeslaus wenzeslaus modified the milestones: 8.4.0, 8.4.1 Jun 10, 2024
@HamishB
Copy link
Contributor

HamishB commented Aug 19, 2024

Who's knowledgeable to review this, and review this in relation to the other half dozen PRs related to the same problem/fixes? I personally didn't follow after the approach changed in one of the d.* PRs, and I'm lost with the discussions scattered.

Hi, that's probably me, although I expect @HuidaeCho is a bit more up to speed than I am on the modern display library. But I'm currently refamiliarizing myself with it, admittedly in an effort to understand some low level weirdness I'm seeing in some of my own code as I port the d.barb module from grass6-addons to grass8 (d.barb is a much enhanced version of d.rast.arrow, and partly based on it). I think I'll have to hunt back in the grass-dev archives from August 2008 to see if @glynnc ever fully described the 'new' source → destination rendering philosophy at a high level and if so add that into the Doxygen comments.

n.b. my usual first stop is to revisit the very well written GRASS 5 Programmer's Manual PDF to see how the original library design was intended so at least I have a solid starting point to understand the changes that followed.

regards,
Hamish

@github-actions github-actions bot added C Related code is in C module labels Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C Related code is in C display GUI wxGUI related module raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants