d.mon output overwrite

[martinl]

d.mon checks output file only if output option is given

d.mon cairo out=map.png
ERROR: option <output>: <map.png> exists.

but not when using default file name of given display driver.

d.mon cairo

[wenzeslaus]

This applies to any d.* command which is drawing something.

Start GRASS and use in terminal:

d.rast elevation

map.png now contains image of elevation raster.

d.vect streams

map.png now contains image of streams vector.

You don't need to call d.mon in order to write to a file named map.png. So, I'm not sure if what you are reporting, is an bug. It is perhaps just undocumented feature. The unchecked overwrite is a bit inconsistent but it also makes sense in this case. I recently learned about this behavior in ​discussion related to r63227 - in grass/trunk/scripts (​nabble, ​gmane).

I don't have any idea what should be the right behavior. It would be good to have different use cases to understand how it is actually used (or supposed to be used).

[martinl]

Replying to wenzeslaus:

This applies to any d.* command which is drawing something.

This ticket is just related to d.mon (when you start a new monitor).

Start GRASS and use in terminal:

d.rast elevation

map.png now contains image of elevation raster.

d.vect streams

map.png now contains image of streams vector.

map.png contains both raster and vector map, which is OK.

You don't need to call d.mon in order to write to a file named map.png. So, I'm not sure

I am not sure what you are referring to. Anyway d.mon out= checks if the file exists. If --overwrite is given than the file is overwritten. If you use --overwrite together with -u flag that the file is open 'update' mode, ie. new data are rendered on the top of the existing file.

This ticket is related to the fact that d.mon behaves differently when launched without output option (when using default display driver output file).

[neteler]

Replying to wenzeslaus:

You don't need to call d.mon in order to write to a file named map.png.

Is this a known feature and the default? I just discovered this recently, especially since the d.* modules do not print a message that "map.png" was generated. So I find these files scattered all over my disk :-)

In essence, is the default driver the cairo or png driver, and controlled by the variables listed in

​http://grass.osgeo.org/grass70/manuals/variables.html#list-of-selected-grass-environment-variables-for-rendering

?

[wenzeslaus]

Replying to neteler:

So I find these files scattered all over my disk :-)

This might be something different but probably related. Call of d.rast when d.mon wx0 is active causes creation of map.png in the current directory:

> ls map.png
ls: cannot access map.png: No such file or directory
> d.mon wx0
> ls map.png
ls: cannot access map.png: No such file or directory
> d.rast elevation
> ls map.png
map.png

I observe this for a long time already but until recently I wasn't able to find a cause. No further investigation done, however.

[neteler]

Replying to wenzeslaus:

Replying to neteler:

So I find these files scattered all over my disk :-)

This might be something different but probably related. Call of d.rast when d.mon wx0 is active causes creation of map.png in the current directory: I observe this for a long time already but until recently I wasn't able to find a cause.

I tried as well - must be display driver related of course. A rather annoying problem...

[martinl]

Summary: originally reported bug has been fixed in r63466 and related commits (r63467 and r63465).

ls map.png ; d.mon cairo
map.png
ERROR: option <output>: <map.png> exists.

d.mon cairo --o
WARNING: File 'map.png' already exists and will be overwritten
Output file: /opt/src/grass_trunk/map.png

[martinl]

Replying to neteler:

I tried as well - must be display driver related of course. A rather annoying problem...

yes, it comes from D_close_driver(), will look at it ASAP.

[martinl]

Replying to neteler:

This might be something different but probably related. Call of d.rast when d.mon wx0 is active causes creation of map.png in the current directory: I observe this for a long time already but until recently I wasn't able to find a cause.

I tried as well - must be display driver related of course. A rather annoying problem...

related to this issue something changed in display architecture (probably Glynn will know more), now you don't need to start any monitor to render data

d.mon -p
No monitor selected

d.vect streams --v
Using display driver <cairo>...
cairo: collecting to file 'map.png'
cairo: image size 640x480
Plotting...
8554 lines plotted
d.vect complete.  

I would see this behaviour as misleading from the user point of view. In GRASS 7.0:

d.mon -p
No monitor selected
d.vect streams 
ERROR: No graphics device selected. Use d.mon to select graphics device.

[martinl]

Replying to martinl:

Replying to neteler: related to this issue something changed in display architecture (probably Glynn will know more), now you don't need to start any monitor to render data

it's r63361 (btw, the log doesn't explain why it bogus...). I really don't see the current behaviour of display commands in trunk as reasonable. The data are rendered automatically by default display driver without any noticing to the user. I strongly suggest to change it back as it works in G70 - to require to start any monitor - otherwise the user will very confused. As we are now.

[glynn]

Replying to martinl:

related to this issue something changed in display architecture (probably Glynn will know more), now you don't need to start any monitor to render data

it's r63361 (btw, the log doesn't explain why it bogus...).

It's bogus because there's simply no reason for that check to be there. Everything worked fine before someone mistakenly added it, and works fine now that it has been removed. The only consequence of that check was to generate an error for entirely valid usage.

I really don't see the current behaviour of display commands in trunk as reasonable.

The current behaviour is exactly the same as the old behaviour before someone decided to try to reincarnate the "monitor" concept from prior versions and introduced a bug in the process.

The data are rendered automatically by default display driver without any noticing to the user. I strongly suggest to change it back as it works in G70 - to require to start any monitor - otherwise the user will very confused. As we are now.

Just because someone managed to introduce a bug which went un-noticed for 3 years, that doesn't make it correct behaviour. GRASS 7.0 hasn't been released yet, so there's no imperative for compatibility.

To re-iterate: GRASS_RENDER_IMMEDIATE is not required for normal operation (generating image files). It isn't supposed to be required, it wasn't required until someone introduced a bug (probably because they thought that it was required), it is no longer required now that the bug has been fixed.

[neteler]

Replying to glynn:

Replying to martinl:

I really don't see the current behaviour of display commands in trunk as reasonable.

The current behaviour is exactly the same as the old behaviour before someone decided to try to reincarnate the "monitor" concept from prior versions and introduced a bug in the process.

This happens.

However, the monitor concept is essential also for GRASS 7. At least for power users who stick to the command line. It is sad enough that the fast x0 monitor from GRASS 6 hasn't been replaced with a fast alternative.

The current behaviour of d.* to silently write into a PNG file is unhelpful. At least a message about the newly generated/modified file should be generated, otherwise the d.* behaviour remains obscure.

[martinl]

Replying to glynn:

It's bogus because there's simply no reason for that check to be there. Everything worked fine before someone mistakenly added it, and works fine now that it has been removed. The only consequence of that check was to generate an error for entirely valid usage.

well, it worked fine also before you reverted it. The reason of this condition was simple. Not to generate output by the default display driver (ie. to write map.png to the current directory) without noticing to the user. Such behaviour is really obscure. The user should be asked to start monitor (d.mon) or to specify display driver by environmental variable (GRASS_RENDER_IMMEDIATE).

[martinl]

Replying to glynn:

The current behaviour is exactly the same as the old behaviour before someone decided to try to reincarnate the "monitor" concept from prior versions and introduced a bug in the process.

please could you elaborate which kind of bug you fixed? From that you wrote it seems to me that you "reincarnate" old behaviour - to silently produce output from display driver without asking user or even notifying him/her.

[glynn]

However, the monitor concept is essential also for GRASS 7. At least for power users who stick to the command line.

Why? Someone who "sticks to the command line" won't even have wxGUI running, in which case d.mon etc don't do anything for them.

The current behaviour of d.* to silently write into a PNG file is unhelpful.

Yet, that's how those commands behaved since the display system was re-written. I don't recall any discussion about the behaviour; it got changed by accident and now that's how it's "supposed" to be? When did we agree to this change, and why?

[neteler]

Replying to glynn:

However, the monitor concept is essential also for GRASS 7. At least for power users who stick to the command line.

Why? Someone who "sticks to the command line" won't even have wxGUI running, in which case d.mon etc don't do anything for them.

OK, then let's call it a "hybrid use mode": Doing everything from command line including opening and closing the wx0/wxN monitor and displaying. Just for map queries and zooming it is handy to use the mouse as in GRASS 6.

The current behaviour of d.* to silently write into a PNG file is unhelpful.

Yet, that's how those commands behaved since the display system was re-written.

Maybe that was the idea but for sure not clearly communicated or implemented in all d.* modules.

I don't recall any discussion about the behaviour; it got changed by accident and now that's how it's "supposed" to be? When did we agree to this change, and why?

Now which change? I just asked that the d.* modules please say what they do since I don't check the file system for a new file every time I issue a command.

If it matters, more people are interested in a working d.mon/command line approach: see #1719,#2233, #2234 and many email discussions in the mailing lists (or ​http://gis.stackexchange.com/questions/71124/is-d-mon-xn-deprecated-in-grass-in-favor-of-something-in-the-wxgui or elsewhere).

[martinl]

Replying to martinl:

Summary: originally reported bug has been fixed in r63466 and related commits (r63467 and r63465).

For record: backported to relbr70 in r63533.

[glynn]

Replying to neteler:

The current behaviour of d.* to silently write into a PNG file is unhelpful.

Yet, that's how those commands behaved since the display system was re-written.

Maybe that was the idea but for sure not clearly communicated or implemented in all d.* modules.

I'm not sure how it could have been communicated more clearly. And the behaviour is consistent across all display[*] modules. It cannot be otherwise, as the behaviour comes from lib/display, not any particular module.

[*] I'd like to say "all d.* modules", but there are a number of cases where people have used that prefix for modules which are unrelated to the display system, e.g. d.mon, d.out.file, etc.

I don't recall any discussion about the behaviour; it got changed by accident and now that's how it's "supposed" to be? When did we agree to this change, and why?

Now which change?

r46984 and r46999.

When the support for 6.x-style monitors was removed in r32584, immediate rendering became the only supported mode of operation (as was already the case on Windows). Thus, it was no longer necessary to set GRASS_RENDER_IMMEDIATE; executing any display command would simply generate an image file (default map.png, configurable via $GRASS_PNGFILE, later changed to $GRASS_RENDER_FILE).

r46984 and r46999 re-introduce a form of the "monitor" concept (although I'm not clear on the details, it has something to do with $MONITOR). AFAICT, r46984 effectively disabled direct rendering. r46999 re-enabled it, but forced $GRASS_RENDER_IMMEDIATE to be explicitly set (if neither $MONITOR nor $GRASS_RENDER_IMMEDIATE are set, it generated a fatal error).

In case it wasn't clear from ​this thread, the requirement for GRASS_RENDER_IMMEDIATE to be set was news to me (I had explicitly set it while investigating differences between the cairo and PNG drivers, and forgotten to remove the setting).

I just asked that the d.* modules please say what they do since I don't check the file system for a new file every time I issue a command.

You were unaware of immediate rendering? Or of it having been the default since the earliest days of GRASS 7?

If it matters, more people are interested in a working d.mon/command line approach:

I don't doubt it. I've tried to have discussions on such things in the past. But some people prefer to just implement hacks like $MONITOR without any discussion. Ultimately, that's likely to be counter-productive in terms of any reasonable solution.

FWIW, I don't actually object to requiring $GRASS_RENDER_IMMEDIATE to be set. I do object to it being done by /fait accompli/. I'm also less than keen on the way that wxGUI seems to be slowly getting shoehorned into somehow being "part of" the display system (e.g. using the d.* prefix for commands which have nothing to do with the display system per se).

If the wxGUI developers need additional functionality from the display system, that should be resolved through discussion rather than "edit wars". Otherwise, it's likely to result in a display system which is of no use for anything but wxGUI.

[martinl]

Replying to glynn:

If it matters, more people are interested in a working d.mon/command line approach:

I don't doubt it. I've tried to have discussions on such things in the past. But some people prefer to just implement hacks like $MONITOR without any discussion. Ultimately, that's likely to be counter-productive in terms of any reasonable solution.

FWIW, I don't actually object to requiring $GRASS_RENDER_IMMEDIATE to be set. I do object to it being done by /fait accompli/. I'm also less than keen on the way that wxGUI seems to be

Let's start with fundamental points:

• some users like monitor concept (GUI monitors, and "file-based" monitors), we should keep it since there is relatively strong request to have them
• direct rendering is a nice feature, but should be enabled only on demand (eg. when $GRASS_RENDER_IMMEDIATE is set), otherwise the most of users will be confused why display modules are producing map.png (or map.ps...) files in the current directory just without asking

Can we have consensus on these points above? If so let's focus on implementation of the second issue and after releasing 7.0 we can discuss better implementation of monitors rather than using hacks like $MONITOR. Make sense to you?

[glynn]

Replying to martinl:

• direct rendering is a nice feature,

It's a bit more than that, given that it's the only way to actually get graphics out of GRASS.

Can we have consensus on these points above?

r63747 requires that either MONITOR or GRASS_RENDER_IMMEDIATE be set. Additionally, GRASS_RENDER_IMMEDIATE=default can be used to select the default driver (cairo if available, otherwise PNG).

If so let's focus on implementation of the second issue and after releasing 7.0 we can discuss better implementation of monitors rather than using hacks like $MONITOR. Make sense to you?

Personally, I'd rather see it done before 7.0 so that it doesn't become something we're stuck with supporting for the next decade. Getting rid of legacy cruft was supposed to be a key point of 7.0, and now we have new cruft added before it's even released.

I would rather see something like:

If MONITOR is set, it names a program which will be executed with the complete argument list of the original program (including argv[0]). So e.g. if MONITOR is set to "wx-monitor", then running "d.rast elevation.dem" would execute "wx-monitor d.rast elevation.dem" then terminate (or would execute it using execve(), which has essentially the same result).

IOW, MONITOR simply causes the display library to delegate everything to some other program. This could write to a file or communicate with wxGUI via a socket (or DDE etc) or whatever. This keeps the details (and any dependencies, which might be platform-specific) out of the display library.

Future changes or extensions wouldn't require any modifications the display library, which hopefully guarantees that the important case (i.e. immediate rendering, which is where everything ends up eventually) won't get broken by such changes.

[martinl]

Replying to glynn:

r63747 requires that either MONITOR or GRASS_RENDER_IMMEDIATE be set. Additionally, GRASS_RENDER_IMMEDIATE=default can be used to select the default driver (cairo if available, otherwise PNG).

I took liberty to do backport including related changes to relbr70 as r63753

[wenzeslaus]

Hi Glynn and Martin, one of the biggest problems of the old state was that it was not documented, so it was not clear what are the possibilities and common usage patterns. Can you please documented as much as you can now? For example, it would be good to document how to effectively use the system from Python scripts and from external applications such as QGIS (which may not posses features such as d.rast.num). I think the proper place would be displayintro.html file.

[martinl]

Replying to glynn:

If MONITOR is set, it names a program which will be executed with the complete argument list of the original program (including argv[0]). So e.g. if MONITOR is set to "wx-monitor", then running "d.rast elevation.dem" would execute "wx-monitor d.rast elevation.dem" then terminate (or would execute it using execve(), which has essentially the same result).

I have implemented that in r64401 and document it in r64403. BTW, now we could remove D_save_command() from trunk which is after this modifications empty, any objections? Currently it's working only for file-based monitors (cairo, png), implementation for wxGUI monitors is in progress.

I have update d.mon which writes currently only one variable, ie. MONITOR. Support files including python renderer are stored in .tmp dir. Also d.erase and d.redraw have been updated to the new architecture.

Testing welcomed.

[glynn]

Replying to martinl:

If MONITOR is set, it names a program which will be executed with the complete argument list of the original program (including argv[0]). So e.g. if MONITOR is set to "wx-monitor", then running "d.rast elevation.dem" would execute "wx-monitor d.rast elevation.dem" then terminate (or would execute it using execve(), which has essentially the same result).

I have implemented that in r64401

Not really. The main issues are:

1. It adds yet more cruft rather than simplifying. Hopefully this is just a transitional phase until existing modules are updated.

2. The arguments are being concatenated into a string (without correct quoting) then passed as a single argument. The output from G_recreate() command may be sufficient for a human-readable description of the command, but it's not suitable for execution. G_parser() needs to save argv so that it can be retrieved, and D_open_driver() needs to pass the actual argv using G_vspawn_ex().

3. The command is required to be a Python script. It should at least support compiled executables.

[martinl]

Replying to glynn:

I have implemented that in r64401

Not really. The main issues are:

well, it was meant as an initial step, feel free to improve.

1. It adds yet more cruft rather than simplifying. Hopefully this is just a transitional phase until existing modules are updated.

Can you be more detailed?

2. The arguments are being concatenated into a string (without correct quoting) then passed as a single argument. The output from G_recreate() command may be sufficient for a human-readable description of the command, but it's not suitable for execution. G_parser() needs to save argv so that it can be retrieved, and D_open_driver() needs to pass the actual argv using G_vspawn_ex().

3. The command is required to be a Python script. It should at least support compiled executables.

Right.

[wenzeslaus]

Replying to glynn:

2. The arguments are being concatenated into a string (without correct quoting) then passed as a single argument. The output from G_recreate() command may be sufficient for a human-readable description of the command, but it's not suitable for execution. G_parser() needs to save argv so that it can be retrieved, and D_open_driver() needs to pass the actual argv using G_vspawn_ex().

Passing the arguments as actual individual arguments is what is used, e.g. in Python:

python file.py arg1 arg2 arg3
python -c file.py arg1 arg2 arg3
python -m module arg1 arg2 arg3

Passing commands as one string is probably necessary in some cases (e.g. when they are stored into a file or when a module would take more different commands as input) but if it can be avoided, it should be avoided I think.

Anyway, Martin, it seems that this will be quite an improvement if you are heading where I think you are heading.

[glynn]

Replying to martinl:

1. It adds yet more cruft rather than simplifying. Hopefully this is just a transitional phase until existing modules are updated.

Can you be more detailed?

Rather than redefining MONITOR (as suggested in comment:20), it keeps that and adds yet another variable, GRASS_RENDER_COMMAND.

The only thing that's actually required is a mechanism to allow all d.* commands to be "redirected" to another program. And that's all that should be provided by the display library. Everything else should be handled by that other program.

So if you want to retain the existing semantics for MONITOR, all references to it should be removed from the display library. Instead, wxGUI would set GRASS_RENDER_COMMAND to refer to a program or script which implements the MONITOR behaviour. Any future changes to the wxGUI monitor mechanism would only affect wxGUI and its utility programs, not the display systems.

[martinl]

Replying to glynn:

So if you want to retain the existing semantics for MONITOR, all references to it should be removed from the display library. Instead, wxGUI would set GRASS_RENDER_COMMAND to refer to a program or script which implements the MONITOR behaviour. Any future changes to the wxGUI monitor mechanism would only affect wxGUI and its utility programs, not the display systems.

it's not only about wxGUI. The reason for having (GRASS) variable MONITOR is the module d.mon which allows to manage "file-based monitors" (like cairo output) and also standalone wxGUI monitors. This feature is requested by many of GRASS users, there is strong interest to have d.mon in GRASS 7.

[glynn]

Replying to wenzeslaus:

Passing commands as one string is probably necessary in some cases (e.g. when they are stored into a file or when a module would take more different commands as input) but if it can be avoided, it should be avoided I think.

When a d.* command is redirected to another program, its argument list needs to be passed as-is.

If you need to store a "command" in a file, then you need to separate the arguments. There are two options here:

1. Use a NUL byte as the argument separator. On both Unix and Windows, any byte except NUL can occur within an argument. NUL cannot occur within an argument, but can occur within a file. However, if the command is only one item within the file, then you also need some mechanism to indicate the end of the command, which leaves the second option:

2. Use some other byte as the separator and escape or quote argument values so that occurrences of the separator within an argument can be distinguished from argument separators.

Ignoring the situation where the separator (or a character used for escaping or quoting) occurs within an argument is not a "solution". Which is why G_recreate_command() can't be used here.

[glynn]

Replying to martinl:

So if you want to retain the existing semantics for MONITOR, all references to it should be removed from the display library. Instead, wxGUI would set GRASS_RENDER_COMMAND to refer to a program or script which implements the MONITOR behaviour. Any future changes to the wxGUI monitor mechanism would only affect wxGUI and its utility programs, not the display systems.

it's not only about wxGUI. The reason for having (GRASS) variable MONITOR is the module d.mon which allows to manage "file-based monitors" (like cairo output) and also standalone wxGUI monitors. This feature is requested by many of GRASS users, there is strong interest to have d.mon in GRASS 7.

Now that you mentioned that, I decided to take a look at d.mon.

I must have missed the discussion about the implementation of this. Can you tell me where to find it?

[mlennert]

Replying to glynn:

Replying to martinl:

So if you want to retain the existing semantics for MONITOR, all references to it should be removed from the display library. Instead, wxGUI would set GRASS_RENDER_COMMAND to refer to a program or script which implements the MONITOR behaviour. Any future changes to the wxGUI monitor mechanism would only affect wxGUI and its utility programs, not the display systems.

it's not only about wxGUI. The reason for having (GRASS) variable MONITOR is the module d.mon which allows to manage "file-based monitors" (like cairo output) and also standalone wxGUI monitors. This feature is requested by many of GRASS users, there is strong interest to have d.mon in GRASS 7.

Now that you mentioned that, I decided to take a look at d.mon.

I must have missed the discussion about the implementation of this. Can you tell me where to find it?

display/d.mon ?

[glynn]

Replying to mlennert:

display/d.mon ?

I was hoping for something which explained the general idea behind what's going on there.

At first glance, this all looks like something of a hack to avoid more substantial changes to the display system, but I might be overlooking something.

[martinl]

Report issue has been solved already. Closing.