[Bug] Display commands for `d.mon` can take a long time *unexpectedly* to update the `cmd` file when first run. about grass HOT 9 OPEN

Or something like this... In D_open_driver()

    if (**running in a tty OR not in GUI**) { /* Or we still need to check GRASS_REGION
                                                 in case we use non-interactive monitors. */
        D_save_command(G_recreate_command());
        D_close_driver();
        return 1; /* it would be easier to just kill the process here, so we don't have to change anything
                     in display modules, but I'm not sure if that would be a better design */
    }

and in each display module:

    /* Setup driver and check important information */
    if (D_open_driver() == 1) exit(EXIT_SUCCESS);

from grass.

OK, D_open_driver() already exit() on adding a new command line, but the command is spawned before that.

from grass.

I did more testing. Without any changes to display modules, changing this:

    if (p && G_strcasecmp(drv->name, p) != 0)
        G_warning(_("Unknown display driver <%s>"), p);
    G_verbose_message(_("Using display driver <%s>..."), drv->name);
    LIB_init(drv);

    init();

    /* don't run display commands if GRASS_REGION (display extent) is not defined;
       rendering will be done by the display driver */
    if (!getenv("GRASS_REGION")) {
        D_close_driver();
        exit(0);
    }

worked.

from grass.

#3482 now uses a less invasive approach by returning -1 and letting individual modules decide what to do when GRASS_REGION is not defined (D_open_driver() == -1). Without any code changes in display modules (not checking D_open_driver() return value), everything should be the same as before.

from grass.

I can confirm the issue, but the solution with GRASS_REGION does not look right to me because GRASS_REGION is what happens to be used in this context and is used in wxGUI and grass.jupyter, but that's not the only way to render.

Additionally, I actually tested the most bare bones way of rendering and that does not work with the new code (your two relevant PRs combined). The following takes ages with the current code, but with the proposed code, it does not render anything:

export GRASS_RENDER_IMMEDIATE=cairo
export GRASS_RENDER_FILE_READ=TRUE
g.region rows=50 cols=50
rm map.png 
d.rast nidp
d.rast.arrow -a drain_tx type=drainage
echo $?  # 0 reported, but map.png contains only nidp

I must say I don't understand how GRASS_REGION fixes that since g.region does not fix it for me here. Are there two issues? Double rendering going on or double init and the region not being respected? Is that the case?

from grass.

I can confirm the issue, but the solution with GRASS_REGION does not look right to me because GRASS_REGION is what happens to be used in this context and is used in wxGUI and grass.jupyter, but that's not the only way to render.

Additionally, I actually tested the most bare bones way of rendering and that does not work with the new code (your two relevant PRs combined). The following takes ages with the current code, but with the proposed code, it does not render anything:

export GRASS_RENDER_IMMEDIATE=cairo
export GRASS_RENDER_FILE_READ=TRUE
g.region rows=50 cols=50
rm map.png 
d.rast nidp
d.rast.arrow -a drain_tx type=drainage
echo $?  # 0 reported, but map.png contains only nidp

I must say I don't understand how GRASS_REGION fixes that since g.region does not fix it for me here. Are there two issues? Double rendering going on or double init and the region not being respected? Is that the case?

The -a flag for d.rast.(arrow|num) currently means draw arrows and numbers aligned with original raster cells.

I think you meant:

export GRASS_RENDER_IMMEDIATE=cairo
export GRASS_RENDER_FILE_READ=TRUE
g.region rows=50 cols=50
rm map.png 
d.rast nidp
d.rast.arrow drain_tx type=drainage

which worked for me with 2fa11a2.

from grass.

I can confirm that with the latest change the basic ("immediate") rendering works again.

Still, does the rendering actually happen twice? If yes, #3482 seems like fixing this double rendering issue using an implementation detail of the monitor-driven rendering somewhere deep in the monitor system, namely usage of GRASS_REGION. If user defines GRASS_REGION in the terminal, the GRASS_REGION-based test will pass and the situation is the same as what these PRs are trying to address. Is there a different way to tell that a d-command is being used as an interface in d.mon monitors rather than being used for actual rendering of data to a file? Or we say GRASS_REGION is the way and ignore the edge cases?

from grass.

Without any of my related PRs,

• With a monitor, yes, it does double-rendering
  • Once by the monitor's render.py (computational region, this was the original issue) and one more by the GUI's watcher (GRASS_REGION)
  • The display module is called three times: 1) by the user (not rendering anything because D_open_driver() exits), 2) by spawned render.py and 3) by the GUI
• Without a monitor (your test case above), no, it doesn't do double-rendering
  • render.py is not spawned (no attached monitor) and
  • the GUI has nothing to do with it because there is no cmd file to watch at all.
• Within g.gui, I don't think it does double-rendering because it always does immediate rendering (no spawning of any external rendering script) with GRASS_REGION (no slow rendering for a small region) if I'm not wrong.

With my related PRs

• With a monitor, no, it won't do double-rendering anymore if the module developer follows the suggested pattern
  • The process by the user just spawns render.py and exits in D_open_driver()
  • D_open_driver() from a new process by render.py returns -1
  • If the suggested pattern is used, this new process just exits without rendering
  • The GUI triggers this module again with GRASS_REGION and D_open_driver() returns 0
  • Now, the module renders only once

If the return value of D_open_driver() isn't checked, the module's behavior will remain the same as before these PRs (double rendering).

• Without a monitor (your test case above), the behavior should be the same as before (single rendering)
  • render.py is not spawned (no attached monitor) and
  • the GUI has nothing to do with it because there is no cmd file to watch at all.

Please try this. Defining GRASS_REGION from the terminal will render once per module because MONITOR check fails (no render.py spawning) and D_open_driver() returns 0.

export GRASS_REGION="proj: 99; zone: 0; north: 842288.7298103943; south: 842056.1497003906; east: -297381.45183609176; west: -297756.32556617697; cols: 1420; rows: 881; e-w resol: 0.2639955846; n-s resol: 0.2639955846; top: 1.000000000000000; bottom: 0.000000000000000; cols3: 44251; rows3: 41262; depths: 1; e-w resol3: 30; n-s resol3: 30; t-b resol: 1;"
export GRASS_RENDER_IMMEDIATE=cairo
export GRASS_RENDER_FILE_READ=TRUE
rm map.png
d.rast nidp
d.rast.arrow -a drain_tx type=drainage

I also tested GRASS_RENDER_COMMAND:

cat<<EOT>cmd.py
#!/usr/bin/env python3
import os
import sys
from grass.script import core as grass
from grass.script import task as gtask

os.environ["GRASS_RENDER_IMMEDIATE"] = "cairo"
os.environ["GRASS_RENDER_FILE_READ"] = "TRUE"

cmd = gtask.cmdstring_to_tuple(sys.argv[1])
grass.run_command(cmd[0], **cmd[1])
EOT
export GRASS_RENDER_COMMAND=cmd.py
rm -f map.png
g.region rows=50 cols=50
d.rast nidp
d.rast.arrow drain_tx type=drainage

It rendered once.

GRASS_RENDER_COMMAND with GRASS_REGION

cat<<EOT>cmd.py
#!/usr/bin/env python3
import os
import sys
from grass.script import core as grass
from grass.script import task as gtask

os.environ["GRASS_RENDER_IMMEDIATE"] = "cairo"
os.environ["GRASS_RENDER_FILE_READ"] = "TRUE"

cmd = gtask.cmdstring_to_tuple(sys.argv[1])
grass.run_command(cmd[0], **cmd[1])
EOT
export GRASS_RENDER_COMMAND=cmd.py
export GRASS_REGION="proj: 99; zone: 0; north: 842288.7298103943; south: 842056.1497003906; east: -297381.45183609176; west: -297756.32556617697; cols: 1420; rows: 881; e-w resol: 0.2639955846; n-s resol: 0.2639955846; top: 1.000000000000000; bottom: 0.000000000000000; cols3: 44251; rows3: 41262; depths: 1; e-w resol3: 30; n-s resol3: 30; t-b resol: 1;"
rm -f map.png
g.region rast=nidp
d.rast nidp
d.rast.arrow -a drain_tx type=drainage

Worked as expected.

Initially, I thought we can simply exit() from D_open_driver() if called from the terminal so we don't have to make any changes to display modules, but not all of them render. For example, d.info doesn't render anything. That's why I changed exit() to return -1. Its (good?) side effect is, no behavioral changes if no code changes.

Is there a different way to tell that a d-command is being used as an interface in d.mon monitors rather than being used for actual rendering of data to a file? Or we say GRASS_REGION is the way and ignore the edge cases?

I think checking MONITOR and GRASS_REGION should be good enough because GRASS_REGION seems to be mainly designed for this purpose (use is the same as WIND_OVERRIDE. ... This allows programs such as the GUI to run external commands on an alternate region without having to modify the WIND file then change it back afterwards.)

It would be convenient to have IPC (we used to have one many many years ago for UN*X) so the display library can communicate with the GUI, but now we have non-GUI display drivers and immediate rendering.

from grass.

I would be be very careful in limiting GRASS_REGION to displays.

I think checking MONITOR and GRASS_REGION should be good enough because GRASS_REGION seems to be mainly designed for this purpose (use is the same as WIND_OVERRIDE. ... This allows programs such as the GUI to run external commands on an alternate region without having to modify the WIND file then change it back afterwards.)

That might have been the original motivation for adding that functionality, but it is compared to the WIND_OVERRIDE mechanism which is general and it has a general name (it is not called GRASS_DISPLAY_REGION or GRASS_RENDER_REGION).

GRASS_REGION is used in r.import, gui/wxpython/psmap, gui/wxpython/iscatt, and many other places in gui/wxpython besides standard 2D rendering. It is also crucial for workflow(script)-level parallelization and scripting in general. It enables controlling computational region without need to deal with files which brings issues such as cleaning temporary files and race conditions. Since GRASS_REGION is a environment variable, it is passed to subprocesses just like GISRC and the runtime environment needed to run GRASS tools, so it is easy to integrate into scripting and complex workflows. (It actually works so well that I want mask to work the same: #2392.)

from grass.