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.

Sign up for GitHub

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

Jump to bottom

d.redraw: Rewritten in C #3484

Closed
wants to merge 38 commits into from
Closed

d.redraw: Rewritten in C #3484

wants to merge 38 commits into from

Conversation

HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Mar 10, 2024 •
edited

This PR rewrites the d.redraw Python script in C. It works with #3482 to constrain redrawing within the current display extent in the monitor. Without this PR and #3482, the following test can take very long because d.rast.arrow has to draw 772,957,282 (number of non-null cells) arrows across the entire DEM even for a very small display extent in the monitor.

wget https://data.isnew.info/grass/nidp.pack https://data.isnew.info/grass/drain_tx.pack
grass -c epsg:5070 /tmp/grass_test
r.unpack -o nidp.pack
r.unpack -o drain_tx.pack
g.region rast=nidp -ap
d.mon wx0
d.rast nidp
# zoom to a very small region in the wx0 monitor (e.g., 50x50 grid)
d.rast.arrow -a drain_tx type=drainage # takes very long for a small region

#3482 and #3492 address the above issue, and this new PR resolves a similar problem with d.redraw because the d.rast.arrow command will be triggered again by d.redraw from the terminal without GRASS_REGION.

The basic idea is to delegate all rendering to the monitor that knows about the display extent. Modules when run from the terminal don't have that information (GRASS_REGION) that is dynamic and internal to the monitor.

@HuidaeCho HuidaeCho self-assigned this Mar 10, 2024
@github-actions github-actions bot added Python Related code is in Python C Related code is in C HTML Related code is in HTML libraries module docs display labels Mar 10, 2024
@HuidaeCho HuidaeCho mentioned this pull request Mar 10, 2024
Closed
@HuidaeCho
Copy link
Member Author

HuidaeCho commented Mar 10, 2024 •
edited

Related issues #3481 and #3486

@HuidaeCho HuidaeCho added the GUI wxGUI related label Mar 10, 2024
@github-actions github-actions bot removed GUI wxGUI related libraries labels Mar 10, 2024
@HuidaeCho HuidaeCho added the enhancement New feature or request label Mar 10, 2024
@HuidaeCho HuidaeCho changed the title d.redraw: New d redraw in C d.redraw: New d.redraw in C Mar 10, 2024
@HuidaeCho HuidaeCho changed the title d.redraw: New d.redraw in C d.redraw: Rewritten in C Mar 10, 2024
@HuidaeCho HuidaeCho marked this pull request as draft March 11, 2024 05:00
@HuidaeCho HuidaeCho mentioned this pull request Mar 11, 2024
Closed
@HuidaeCho HuidaeCho marked this pull request as ready for review March 11, 2024 08:40
wenzeslaus and others added 11 commits May 21, 2024 06:58
@wenzeslaus @HuidaeCho
grass.script: Fix missing passing of env parameter (OSGeo#3684)
f5846f6
@tmszi @HuidaeCho
wxGUI/dbmgr: fix enclosing column name with SQL standard double quotes (
7789cf3 
OSGeo#3634)

Enable to use (show/add/delete/rename/query) column name which is
reserved DB backend keyword.
@wenzeslaus @HuidaeCho
g.message: Do not require full gisinit (OSGeo#3686)
2a099d9 
The g.message does not need any of the session setup and it can be used during a setup process to report standardized messages to the user. This removes the need to have a full session before running g.message. GISRC is still needed, but the data (file) structures for mapset don't need to exist.

The code is combination of what is in g.proj (the 'no init' call and comment) and g.dirseps (memory mode for GISRC). It is also syncing the corresponding lines in g.proj.
@renovate @HuidaeCho
CI(deps): Update docker/dockerfile:1.7 Docker digest to a57df69 (OSGe…
80f9d4f 
…o#3668)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@wenzeslaus @HuidaeCho
grass.script: Always use full env in create_project (OSGeo#3685)
f345b1e 
For reporting errors the g.message tool is called, so the environment setup is always needed (except when doing XY without errors).
@wenzeslaus @HuidaeCho
grass.script: Get GISBASE automatically in get_commands (OSGeo#3683)
cb2efad 
* grass.script: Get GISBASE automatically in get_commands

The _grass.script.get_commands_ function requires GISBASE (more specifically, path to binary executables and scripts). The need for GISBASE prevents its use before a session (more specifically the runtime part of it) is created. This changes the function to get the GISBASE automatically when not set. It also adds the standard env parameter other functions have (which is useful mainly for tests).

* Add mock for tests
@renovate @HuidaeCho
CI: Update msys2/setup-msys2 action to v2.23.0 (OSGeo#3690)
0ce8a07 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@petrasovaa @HuidaeCho
grass.script: Automatically parse JSON and CSV in parse_command (OSGe…
ba00e43 
…o#3687)
@renovate @HuidaeCho
CI(deps): Update github/codeql-action action to v3.25.4 (OSGeo#3692)
e635c59 
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@renovate @HuidaeCho
CI(deps): Update softprops/action-gh-release action to v2.0.5 (OSGeo#…
61bf2ae 
…3691)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@HuidaeCho
Add back d.redraw because OSGeo#3500 deletes it
95e9be6
@github-actions github-actions bot added GUI wxGUI related Linux Linux specific docker Docker related CI Continuous integration vector Related to vector data processing raster Related to raster data processing temporal Related to temporal data processing libraries general imagery tests Related to Test Suite notebook misc labels May 21, 2024
@HuidaeCho
Copy link
Member Author

Hmm.... I think I broke it... Not sure why it's happening. My local clone looks fine.

@HuidaeCho HuidaeCho mentioned this pull request May 21, 2024
Merged
@HuidaeCho
Copy link
Member Author

Replaced by #3727.

@HuidaeCho HuidaeCho closed this May 21, 2024
@wenzeslaus
Copy link
Member

...Not sure why it's happening...

Might be a pull instead when a force push was appropriate: OSGeo/grass-addons#802 (comment)

To fix broken states, you can also force push a new branch to an old PR. Preserves comment and review history and links from other PRs. The Git mechanics for that is force pushing to a remote branch of a different name (you force push just the first time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@echoix echoix echoix left review comments

Assignees

@HuidaeCho HuidaeCho

Labels
C Related code is in C CI Continuous integration display docker Docker related docs enhancement New feature or request general GUI wxGUI related HTML Related code is in HTML imagery libraries Linux Linux specific misc module notebook Python Related code is in Python raster Related to raster data processing temporal Related to temporal data processing tests Related to Test Suite vector Related to vector data processing
Projects
None yet
Milestone
8.4.0
Development

Successfully merging this pull request may close these issues.

[Bug] d.redraw script doesn't redraw d.rast.leg
9 participants
@HuidaeCho @echoix @neteler @wenzeslaus @tmszi @lindakarlovska @mshukuno @nilason @petrasovaa