search

danieljprice / giza

giza - a scientific plotting library for C/Fortran
https://danieljprice.github.io/giza/
GNU Lesser General Public License v3.0
32 stars 19 forks source link

PGPAP/X-window resize (original issue: Movie mode shortcut stalls Splash) #18

Closed markahutch closed 6 years ago

markahutch commented 6 years ago

The movie mode shortcut no longer works for me. If I press Enter, the page size changes, but nothing appears and I have to force quit Splash.

danieljprice commented 6 years ago

I can reproduce. This is a major bug. However, pretty sure it's a giza bug introduced by the ability to resize X-windows. Should therefore only occur in giza 1.0.0 or higher. Will file corresponding giza bug report also...

danieljprice commented 6 years ago

I confirm this was caused by 55fb9789

I also confirm that this breaks the PGPAP implementation. For example, pgdemo12 no longer works correctly

haavee commented 6 years ago

EDIT: see the update below

The X-Window resize is a bit of a problem :-( Part of this might be alleviated by copying the old pixmap onto the new window so it does not remain blank. Not sure how the "resizing the PGPLOT window in the background" is to be properly done without an X event loop; I've already spend some time thinking about how to do this.

An idea I had was to make the giza X driver paint to a recording surface such that it can be replayed after the window is resized.

haavee commented 6 years ago

Update

I have been investigating why pgdemo12 doesn't seem to work with giza. It may turn out to be deeper/different than just the giza-driver-xw's resizing implementation, although that change may have highlighted an underlying difference between PGPLOT and giza.

pgdemo12 is very simple and included here to be able to illustrate how PGPLOT and giza behave differently.

  WRITE (*,*) 'First page: size is 7 x 3.5 inch'
  CALL PGPAP(7.0,0.5)
  CALL PGENV(0.0,1.0,0.0,2.0,0,0)
  CALL PGLAB('x','y','1')
  WRITE (*,*) 'Second page: size to 6 x 6 inch'
  CALL PGPAP(6.0,1.0)
  CALL PGENV(0.0,1.0,0.0,2.0,0,0)
  CALL PGLAB('x','y','2')
  WRITE (*,*) 'Third page: same size as second'
  ...

What I found is that there is a difference in behaviour when a call's effects are visible.

Now it gets interesting ...

Without the block-for-user-input inserted manually, giza would draw the first example correctly and erase it almost even faster.

Note that the immediate effect of giza's implementation of PGPAP/cpgpap() is due to a call to giza_change_page() in there, which, in turn, calls _giza_change_page_xw(), which 'just does its thing'.

So probably the misbehaving reported here goes deeper than a mere X-window (re)sizing issue.

I can all too easily imagine that differences in immediate-versus-delayed effect taking place after an API call can easily disrupt applications relying on a specific behaviour.

danieljprice commented 6 years ago

some of this might be historical. We did not originally have a working resize implementation, so giza's "PGPAP" had to be called BEFORE the call to giza_open to set the paper size. So this may be an easy fix to restore pgplot-like behaviour by removing the "immediate effect" thing from within Giza's pgpap -- so the page resize does not take effect until the next pgenv call

haavee commented 6 years ago

Can someone test this branch https://github.com/haavee/giza/tree/issue-18 ?

I've made a couple of changes in an attempt to be more PGPLOT compatible:

pgdemo12 now works identical for me using PGPLOT or giza

markahutch commented 6 years ago

If I checkout the giza branch in the link, I get the following error when I try to plot a file with Splash:

At line 70 of file ../src/defaults.f90 Internal Error: get_unit(): Bad internal unit KIND

On Wed, Nov 14, 2018 at 5:34 PM Harro Verkouter notifications@github.com wrote:

Can someone test this branch https://github.com/haavee/giza/tree/issue-18 ?

I've made a couple of changes in an attempt to be more PGPLOT compatible:

  • asking for confirmation now skips the first time (I cheated by looking into PGPLOT source code and add a similar per-device flag + reset of that flag at the same point in the code)
  • the code now prompts before creating a new page; before the code would first change to a new page (erasing what's on screen ...) and only then ask "oh, should I have actually done that?" ;-)
  • pgpap() does not call giza_change_page() anymore; it now only updates the bookkeeping in Dev[id] and sets the .resize flag (which it already did)
  • _giza_change_page_xw() already handled resizing of the window if, upon entry, the Dev[id].resize flag was set. This takes (and took) preference over detecting if someone had changed the size of the XWindow e.g. by dragging with the mouse. Such an event would be picked up on moving to the next page.
  • _giza_change_size_xw() removed because that one acted immediately, which was (a) not desired and (b) it was never called anyway; the act of physically resizing the XWindow + new surface creation was already handled by the updated _giza_change_page_xw() anyway.

pgdemo12 now works identical for me using PGPLOT or giza

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danieljprice/giza/issues/18#issuecomment-438727832, or mute the thread https://github.com/notifications/unsubscribe-auth/AZrFakuenTJHuyaTKPxkc6SPcK3x_Lssks5uvEYVgaJpZM4YVtCB .

danieljprice commented 6 years ago

there's a routine missing, I can't compile the branch

Undefined symbols for architecture x86_64:
  "__giza_change_size_xw", referenced from:
      __giza_resize_device in libgiza_la-giza-drivers.o

in my source code the routine "giza_change_size_xw" does not exist...

haavee commented 6 years ago

Oh bugger. Can you pull my fork and checkout issue-18 again? There was a function still referencing the removed _giza_change_size_xw - should be fixed now.

As to the src/defaults.f90 I don't see that in the giza source tree. Or is that in Splash? To the best of my knowledge I haven't touched any of the internal unit / pgplot-to-giza unit conversion.

markahutch commented 6 years ago

Perhaps I just forgot to 'make clean' first, sorry. I checked out issue-18 again and I still get a hanging white screen when using the movie shortcut.

On Thu, Nov 15, 2018 at 10:22 AM Harro Verkouter notifications@github.com wrote:

Oh bugger. Can you pull my fork and checkout issue-18 again? There was a function still referencing the removed _giza_change_size_xw - should be fixed now.

As to the src/defaults.f90 I don't see that in the giza source tree. Or is that in Splash? To the best of my knowledge I haven't touched any of the internal unit / pgplot-to-giza unit conversion.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danieljprice/giza/issues/18#issuecomment-438973211, or mute the thread https://github.com/notifications/unsubscribe-auth/AZrFajDX9OYNsqqM7pZWxdztSX94KPnrks5uvTJTgaJpZM4YVtCB .

haavee commented 6 years ago

I've just downloaded & built Splash, I'd like to see what's happening. Can you give some details "how to reproduce"? (I've never seen or used Splash, sorry)

markahutch commented 6 years ago

Once Splash is built, you just need a file to read. I have attached a small file that you can use. In the same directory as the file, you should then be able to type:

ssplash dump_00000

This will bring up the main menu with various quantities you can plot. To reproduce the blank screen, you will want a rendered plot with which to start. To do this, you can enter the following values:

2 1 6

You should now have a rendered image with axes and a colourbar. In previous versions, we could simply hit once more and it would automatically adjust the page size, background colours, and axes which were more suitable for making videos. However, now if you hit , it adjusts the page size, but it stalls on a blank white background and you have to force quite Splash to regain control. On Thu, Nov 15, 2018 at 11:21 AM Harro Verkouter wrote: > I've just downloaded & built Splash, I'd like to see what's happening. Can > you give some details "how to reproduce"? (I've never seen or used Splash, > sorry) > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > , > or mute the thread > > . >
haavee commented 6 years ago

Hi Mark, thanks! But ... am I looking in the wrong place? Because I don't see an attachment?

markahutch commented 6 years ago

I definitely attached the file...perhaps the github server doesn't allow attachments? Maybe it is easier if I send you the link to the file in my Dropbox: https://www.dropbox.com/s/ggcmmw250zwxcmb/dump_00000?dl=0 https://www.dropbox.com/s/ggcmmw250zwxcmb/dump_00000?dl=0

On Thu, Nov 15, 2018 at 3:02 PM Harro Verkouter notifications@github.com wrote:

Hi Mark, thanks! But ... am I looking in the wrong place? Because I don't see an attachment?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danieljprice/giza/issues/18#issuecomment-439049903, or mute the thread https://github.com/notifications/unsubscribe-auth/AZrFaqTgU8g1l4UyTlnQvNlNl7wlx-Lrks5uvXQAgaJpZM4YVtCB .

haavee commented 6 years ago

Thanks, got it! It seems that the whole of 'interactive mode' stops working; even typing 'h' doesn't work anymore. But at least now I have something to reproduce+test with!

haavee commented 6 years ago

I think I found it. Can you try https://github.com/haavee/giza/tree/issue-18 again? Don't forget to git pull and make clean && make ...

I traced it to _giza_change_page_xw() waiting for a ConfigureNotify event from the X system after it requests X to resize the window (which is what PGPAP() eventually aims to do). That event never arrived ... so it just sat there in an infinite wait, the only escape out of that being the force-quit.

I've removed the wait-for-event and both pgdemo12 as well as bin/ssplash dump_0000 now work for me.

markahutch commented 6 years ago

Beautiful! Works for me as well. Thanks for all of your work in fixing this issue. It makes it so much easier than having to configure the page manually every time.

Cheers,

Mark

On Fri, Nov 16, 2018 at 6:06 PM Harro Verkouter notifications@github.com wrote:

I think I found it. Can you try https://github.com/haavee/giza/tree/issue-18 again? Don't forget to git pull and make clean && make ...

I traced it to _giza_change_page_xw() waiting for a ConfigureNotify event from the X system after it requests X to resize the window (which is what PGPAP() eventually aims to do). That event never arrived ... so it just sat there in an infinite wait, the only escape out of that being the force-quit.

I've removed the wait-for-event and both pgdemo12 as well as bin/ssplash dump_0000 now work for me.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/danieljprice/giza/issues/18#issuecomment-439460316, or mute the thread https://github.com/notifications/unsubscribe-auth/AZrFamyQMfxD7dWCsjJlARPK7oJ46GIlks5uvvCjgaJpZM4YVtCB .

danieljprice commented 6 years ago

I also confirm this works -- thanks Harro! Will merge.