r.in.poly broken by window split

[hamish]

Hi,

r.in.poly was missed during the split window transition (r42876) and now exits with an error. (previously there was a suppressed-by-the-module warning)

ERROR: Output window changed while maps are open for write

a simple change in the row paging code of Rast_set_window() to Rast_set_output_window() in r.in.poly/raster.c didn't help; any advice on how the input/output split windows now work? the doxygen lib fn comments briefly explain the what, but not the why of the design or how you're supposed to use it.

thanks, Hamish

[hamish]

i.topo.corr in grass7 addons too.

[mmetz]

Replying to hamish:

i.topo.corr in grass7 addons too.

Both r.in.poly and i.topo.corr should be fixed now.

Markus M

[glynn]

Replying to hamish:

any advice on how the input/output split windows now work?

The input window determines the number of rows and columns for Rast_get_row() etc, i.e. the valid range for the "row" parameter, and the number of elements in the "buf" array.

The output window determines the number of rows and columns for Rast_put_row() etc, i.e. the number of times it should be called ("put" operations don't have a "row" parameter) and the number of elements in the "buf" array.

For most modules, the input and output windows should be the same, but e.g. r.resamp.* will typically use split windows.

The rationale was that the old mechanism was inefficient. It would change the window twice for each row of output (set "read" window, read rows, set "write" window, write row). Each change caused the column mapping to be recalculated for each input map. With split windows, there are separate windows for input maps and output maps, eliminating the need to change back and forth between input and output windows. The column mapping is calculated when a map is opened and never changes. Attempting to change the input window while any input maps are open generates an error, as does attempting to change the output window while any output maps are open.

Also, certain functions assume the existence of a single window, e.g. Rast_get_window(), Rast_window_{rows,cols}(). These functions still exist, and work so long as the windows aren't split (i.e. neither Rast_set_input_window() nor Rast_set_output_window() have been called, or the windows have since been joined by calling Rast_set_window()), but will generate an error if the windows are split. If the windows are split, the code must use e.g. Rast_get_input_window(), Rast_input_window_rows() etc to read the appropriate window.

The "split window" design eliminates the most common reason for changing the window while maps are open, although there may be cases it doesn't cover (e.g. reading multiple input maps at different resolutions won't work). If we need to support that, there are a number of possible solutions.

One is to store the current window in the fileinfo structure whenever the column mapping is created, check that it matches the current window whenever Rast_read_row() etc is called, and either re-generate it or generate an error if it differs. This avoids having to needlessly re-calculate the column mapping for all input maps on every set-window operation, but requires a window comparison (which probably needs some tolerance for comparing floating-point fields) for each get-row operation.

Another is to allow each map to have a separate window, set from the current window when the map is opened. The main drawback is that Rast_window_rows() etc become meaningless. We would need to add yet another "level" of window splitting, so you'd have: single window, read/write windows, per-map windows.

As usual, the main problem isn't implementation, but design and ensuring that existing code doesn't silently break (the current implementation should ensure that any breakage results in a fatal error).

[neteler]

Please add a synthesis in lib/raster/rasterlib.dox

[neteler]

Replying to neteler:

Please add a synthesis in lib/raster/rasterlib.dox

Done in r59331 (verification in SVN recommended).

[hamish]

Replying to mmetz:

Both r.in.poly and i.topo.corr should be fixed now.

Hi,

fyi I still get the "ERROR: Output window changed while ..." error with r.in.poly with the latest trunk. I used the example from the man page piped through stdin.

thanks, Hamish

[hamish]

g.region e=591450 w=591300 n=492660 s=492600 -p

r.in.poly in=- out=test1234 << EOF
A
   591316.80   4926455.50
   591410.25   4926482.40
   591434.60   4926393.60
   591341.20   4926368.70
= 42 stadium
EOF

[wenzeslaus]

I don't get any error with the code posted and G7:r.in.poly changed quite a bit. Closing the ticket as fixed. Reopen if you are still getting ...Output window changed while... and please submit ​a test which shows it.