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.

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

GTK2 End of Life #111

Closed
yourealwaysbe opened this issue Dec 16, 2020 · 63 comments
Closed

GTK2 End of Life #111

yourealwaysbe opened this issue Dec 16, 2020 · 63 comments

Comments

@yourealwaysbe
Copy link
Contributor

GTK4 was announced today, along with the end of GTK2.

I know switching Viking away from GTK2 isn't easy -- i tried and failed myself several years ago. I'm putting this issue up in case anyone has any ideas or enthusiasm for moving things forward.

@rnorris
Copy link
Collaborator

rnorris commented Dec 29, 2020

Just some heads up about the work I'm doing on this.

ATM I have a functional base build, i.e. basically all the drawing is disabled
This gets the basic widgets working (with only a slight change for some GtkVBox / GtkHbox -> GtkBox).

On top of this I've reworked the Track property graphs (elevation/distance etc...) based on cairo. This works well - once I understood some subtleties of signalling (and more importantly on which widgets it is best on), compared to the original version.

The main difficulty is probably the main viewport draw.
In the original version everything is drawn into several GdkPixmaps and GdkGCs and ends up composed together.
Hence the use of alpha blending in merging various maps on top of each other and getting multiple TrackWaypoint layers shown.
It seems reasonably straight-forward to get a single map (in progress but not working properly yet...) and/or a single Track drawn. However since the GdkPixmap and GdkGC types are removed, I'm not sure of the way to handle multiple layers.
Maybe need to save the equivalent cairo type - such as from gdk_window_create_similar_surface(), and repeatedly draw to that, or perhaps something more crazy such as make each layer have it's own GtkDrawingArea that it draws to but then we have the extra complication of having to manage the size of it for each layer.

I will aim to upload my current work in a branch that is liable to be force reset at any time for general criticism :)

ATM it will aim to allow building in either GTK2 or GTK3 variant controllable via .configure option "--enable-gtk3"

@yourealwaysbe
Copy link
Contributor Author

Thanks for this -- i also came unstuck on the Cairo stuff with pixmaps and GCs -- though i had/have little understanding of them or how to use Cairo.

@rnorris
Copy link
Collaborator

rnorris commented Dec 31, 2020

Here is the current usable work:

https://github.com/rnorris/viking/tree/GTK3

To switch from an existing default GTK2 build you need to clean first otherwise you may get linking errors, e.g.:

make clean && ./autogen --enable-gtk3 && make

@yourealwaysbe
Copy link
Contributor Author

Great -- i can confirm it compiles and runs (without the main viewport) for me, if that's any help :)

@yourealwaysbe
Copy link
Contributor Author

Based on my superficial knowledge, would the GTK4 node rendering model be suited to rendering tracks/routes/waypoints?

@rnorris
Copy link
Collaborator

rnorris commented Dec 31, 2020

ATM I understand the GTK4 Scenegraph stuff even less.
Plus updating to GTK4 means needing to work out a development system and a plan for the Windows build (and Flatpak...) - at least for GTK3 the upstream dependencies are available and so very easy to change to - GTK4 maybe currently may be a bit too new (?).

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Dec 31, 2020

That's fine, just a thought.

I played a bit today with compiling against GTK4 instead of GTK3. I guess predictably i've run into bits where i'd need to understand the Viking code better and the intention of GTK4. I'm not that familiar with GTK in general or Viking beyond a few specific parts, so that doesn't help.

It also doesn't help that the GTK4 documentation seems to have a lot of dead links. Perhaps GTK3 makes sense as a step first while GTK4 settles and more advice is available about porting from GTK3.

The main stumbling blocks so far seem to be:

  • GtkStatusbar has been made final, so VikStatusbar cannot extend it.
  • GtkRadioActionEntry has been removed. I haven't really figured out the intended replacement. There's a vague comment about it in the GTK docs, but it didn't help me.
  • GtkMenu is gone.
  • GdkPoint is also removed, but i expect that goes with the move to Cairo or Scenegraphs.

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 1, 2021

In the original version everything is drawn into several GdkPixmaps and GdkGCs and ends up composed together.

Is it possible to compose GdkPixbufs (with gdk_pixbuf_composite) and then use cairo_set_source_pixbuf? Though i'd expect there's a more purely Cairo way of doing it.

edit: though i expect this only works for combining images, not for drawing tracks &c. onto them.

editedit: after gaining some understanding of Cairo i see that it works by setting a source, creating a mask, and then painting the source onto a surface. You can change source and paint again on the same surface, so drawing tracks on top of maps should be doable, with the Viking viewport holding the one surface - maybe? To alpha-blend map tiles, could you use cairo_paint_with_alpha or this technique from StackOverflow?

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 2, 2021

In the original version everything is drawn into several GdkPixmaps and GdkGCs and ends up composed together.

I've had a play with Cairo and have a proof of concept of a few ideas here. The official docs also has a nice list of samples.

I'm thinking that there would be two potential models:

  1. Have one main cairo_t in vik_viewport and provide some drawing functions (to draw images, lines, points) that other parts of the code can use.
  2. Have a main cairo_t in vik_viewport and provide a function to copy another Cairo surface onto the viewport. Then Cairo surfaces might take the place of GdkPixmaps.

Perhaps cairo_t takes over from GdkGC but i'm not sure it will provide all features (or a "VikGC" is defined to take its place).

The proof of concept code tries three things:

  • Draw a couple of images with an alpha value (actually, images are loaded as Cairo surfaces so this is both models 1 and 2).
  • Draw a line directly onto a main surface.
  • Draw a line indirectly by first drawing onto a local surface, then plastering the local surface on top of the main one.

@rnorris
Copy link
Collaborator

rnorris commented Jan 2, 2021

So a recap on how the drawing works in Viking (as per my understanding):

  1. When a new window is created an internal pixmap is created the size of the viewport
  2. If a window is resized we track configure event to resize the pixmap to the new size.
  3. Expose events cause the internal pixmap to be drawn on to the drawing area.
  4. When the user does something that results in visible change to the screen (e.g. loading a track), the code issues an internal 'layer update' signal (which maybe in a background thread). This signal is received by the vik layers panel and re-emits an update signal but always into the primary thread. This signal is then in turn received by vik window which then calls a 'draw_update' which in turn calls back into the vik layer panel, causing every layer to draw on to the viewport which uses the internal pixmap.
  5. After the layer drawing, the vik window calls the viewport sync which causes the internal pixmap to be drawn on to the drawing area.

There is some additional complication of 'trigger layers' / snapshot / half-drawn. Which to be honest I've never properly understood. I think it attempts to use a pixmap of an aggregate type layer, such that if the layer that emitted the update is above the layer being requested to redraw, it can skip the drawing and use the existing pixmap since it would not have changed.
I'm not entirely convinced this works (in saving time to redraw things) and so may not worth trying to emulate in the brave new world.

Things to note:
You get many expose (draw) events, so you do not actually want to perform the recalculation of all the layer draws inside the expose event. Hence that's why ATM the redraw is only performed when the actual layer has changed.

I've been also trying/thinking of saving a cairo_t * in the vikviewport as an equivalent of the GdkPixmap - using gdk_cairo_create(), specifically to draw outside of the paint cycle - although the GTK3 manual says:
"Typically, this function is used to draw on a GdkWindow out of the paint cycle of the toolkit; this should be avoided, as it breaks various assumptions and optimizations."

I hadn't successfully managed to get the screen to draw though, but hopefully seeing you usage of cairo_set_source_surface() - that might be the secret to getting it to work...

Note that the use of GdkGCs was seeming mainly to enable different colours and sometimes line widths. Using cairo line drawing you can change the colour anytime. As done in the new track properties graph drawings.

Using cairo it seems can't draw individual pixbufs into a specifc area - only a whole image. So I'm in the process of shifting the map layer to have a pixbuf the full screen size and using gdk_pixbuf_copy_area() to construct from each tile and then only draw to screen the full pixbuf. Something similar will need to be done for waypoint image icons / thumbnails, the mapnik layer, the DEM layer and the georef layer.

Still may have to use gdk_pixbuf_composite() on all these layer pixbufs. Hopefully this is not unusably slow.

@yourealwaysbe
Copy link
Contributor Author

[Viking summary]

Thanks for this -- i'm generally ignorant of the way Viking works. Most of my posts here are trying to use the odd hour i have to do something slightly more useful than asking you to do it :)

I've been also trying/thinking of saving a cairo_t * in the vikviewport as an equivalent of the GdkPixmap - using
gdk_cairo_create(), specifically to draw outside of the paint cycle - although the GTK3 manual says:
"Typically, this function is used to draw on a GdkWindow out of the paint cycle of the toolkit; this should be avoided, as it breaks various assumptions and optimizations."

Why not use cairo_create directly? I'd assume that gdk_cairo_create does specific stuff for drawing onto a window rather than a general cairo_surface_t, which could be targeting anything (and independent of GTK). The general surface could then be copied onto the viewport during a redraw event.

Using cairo it seems can't draw individual pixbufs into a specifc area - only a whole image.

I'll admit to not really appreciating the differences between pixbufs and pixmaps, but i think you can copy parts of a cairo surface to another. Are pixbufs specifically needed?

@rnorris
Copy link
Collaborator

rnorris commented Jan 3, 2021

So in the end pixbufs worked much more easily and they do get composited on to each other, so no don't have to create the whole image (and neither having to manually composite together).

In the end didn't use gdk_cairo_create() (nor gdk_window_create_similar_surface() - as for whatever reason they didn't really work. Hence going off down rabbit holes trying random stuff in a vain effort to get something working...

Pixbufs are the standard GTK way to handle images formats including to/from disk (JPG/PNG etc...)

ATM now getting the various draw_line (rectangle/polygon +arc) features to work - which all seem very doable.
The basic draw line is working, but wondering about how clean one can keep the code between GTK2 and GTK3 versions.
Unclear is whether one can (or is best to) create and manage multiple cairo_t for the various colours and line width options, or just switch the colour / style as needed.

However ultimately do want to have a sort of transparent layer in order to draw the 'new line' of the ruler/zoom bounds/new track, as this line is not permanent, but follows the mouse pointer location.

Force Updated:
https://github.com/rnorris/viking/tree/GTK3

@gdt
Copy link

gdt commented Jan 3, 2021

My take is that if you get a tree that builds and works well on GTK3, it's perfectly ok to just merge it and drop GTK2 support. I do not think there is any need to accomodate systems with gtk2 and not gtk3.

@yourealwaysbe
Copy link
Contributor Author

So in the end pixbufs worked much more easily and they do get composited on to each other, so no don't have to create the whole image (and neither having to manually composite together).

Excellent, works well for me on Wayland with Sway (not using xwayland). The behaviour when map tiles are not yet downloaded is a bit crazy -- let me know if you don't see that already and i'll post a screenshot.

@rnorris
Copy link
Collaborator

rnorris commented Jan 4, 2021

Post the screen shot please. ATM I believe I've only run it with map tiles already downloaded.

@yourealwaysbe
Copy link
Contributor Author

Attached. It draws nicely once the tiles are downloaded, but it does make it hard to quickly navigate new areas.

Screenshot
)

@rnorris
Copy link
Collaborator

rnorris commented Jan 4, 2021

I think this is due to drawing a tile from a different zoom level, when the zoom level desired is unavailable.

The old way of drawing a tile specified the full x1,y1,x2,y2 - so parts of tiles could be drawn - probably auto scaling as necessary; whereas now it only draws at native scale. So for the try_draw_scale_down/up() methods it might now need to perform scaling and/or copy_area from the source pixbuf manually.

Edit: Now fixed - but I've yet upload it, as its not too critical.

@roooots
Copy link

roooots commented Jan 14, 2021

Hi,

I've just built viking from git using ./autogen.sh --enable-gtk3 and it appears to be working fine so far (loading various maps, drawing tracks, ...) - congrats and thanks for the efforts! :-)

However, the reason why I came across this thread is that I'm currently toying around with XFCE4 under Ubuntu 20.04 on a 4K display. While most apps are scaling fine, GTK2 apps don't, giving me tiny little fonts. Now when building viking with GTK3 I expected that the UI fonts would scale nicely, but unfortunately they don't - they're the same size as with the GTK2 version. Any suggestions why that could be? Would that need a rework of the UI as well?

Unfortunately, I hardly have any coding skills, but if I can assist in debugging or other ways, please let me know!

Cheers,
r.

@yourealwaysbe
Copy link
Contributor Author

@roooots track drawing currently does not work on GTK3. Is it possible that you are still running the GTK2 version? Perhaps doing "make clean" before the autogen and compile would sort it.

@roooots
Copy link

roooots commented Jan 15, 2021

Thanks for that pointer, I must have mixed something up with the downloaded vs. git code, I was running the GTK2 version.

However, when trying to build the latest code downloaded from https://github.com/rnorris/viking/tree/GTK3 with GTK3 enabled I'm getting an error:

vikviewport.c: At top level: vikviewport.c:1491: error: unterminated #else

I solved this by adding an #endif in line 1521 of vikviewport.c. The build completes without errors and viking obviously runs as GTK3 test build. I'm still seeing the same tiling effect you reported with the screenshot, but as soon as all tiles have been downloaded, drawing is ok. With my 4K display, however, the tiles obviously are not drawn in native resolution even with OSMAND HD. The resolution is higher than with default Mapnik, but not native.

Cheers,
r.

@roooots
Copy link

roooots commented Jan 16, 2021

A brief follow-up regarding my original question about font scaling: with the GTK3 build, font scaling is correct. To get proper font scaling with the GTK2 version, I've modified ~/.themes/mythemename/gtk-2.0/gtkrc and added the following lines:

style "font"
{
font_name = "Corbel 22"
}
widget_class "*" style "font"
gtk-font-name = "Corbel 22"

Font name and size may be chosen to one's liking.

Edit: unfortunately line breaks are not properly shown here in the code environment, so I replaced it by normal text.

Cheers,
r.

@rnorris
Copy link
Collaborator

rnorris commented Jan 16, 2021

A provisionally complete version is here:

https://github.com/rnorris/viking/tree/GTK3-WIP

However I will rework these hodge podge of commits into hopefully something a bit more individually understandable.

The main takeway is that there are two sets of cairo references and surfaces in the viewport.
The primary one is used for 'static' image of the pixmaps and tracks.
There is a secondary one that used for drawing volatile things that respond to the mouse movements (i.e. ruler tool, new track/route point and a few other things).
These are combined together in the vik_viewport_sync() performed in the main GTK widget draw callback.

Further things to note a couple of hacky type definitions enables keeping much drawing code the same or similar for GTK2 & GTK3 versions.

However the lifecycle of the cairo references & surfaces is complicated as seemingly there is no way to resize them and to get it to show on screen it must be the one used in the viewport widget draw callback; whereas the GTK2 one could force a draw of any pixmap.

As layers typically copy/get a reference to this type - they need to know when it has changed - normally as the result of configure events - often due to the window being resized.

There are lightly to be many obscure bugs in the lesser used features.
I haven't tried it with the georef, mapnik or gps layers at all yet.

@yourealwaysbe
Copy link
Contributor Author

Very nice -- from a quick play it looks good. I'll start using it and see if i spot any bugs.

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 17, 2021

I spotted a scrolling error. At least on Wayland, the touchpad scroll does not work. There's a discussion about this here that seems to narrow it down to touchpads only sending smooth scroll events on Wayland. There is some discussion about X sending both that may or may not cause issues.

I attach a diff that does a naive fix. This seems OK on Wayland with both touchpads and mousewheels, and also with GDK_BACKEND=x11.

The diff also "fixes" a bug with touchscreen dragging on Wayland. On GTK2, and GTK3 with X11, you can move the map around by dragging on the touchscreen. This does not work on Wayland, and it turns out to be because of some code in draw_mouse_motion that is labelled as a hack with a bug reference. Commenting the hack out seems to correct it with both Wayland and X11 backends -- perhaps the bug it was addressing is no longer relevant?

@yourealwaysbe
Copy link
Contributor Author

I'm not sure if it's just me, but it seems to download a lot of maps? In fact, it downloads 27 items every time i open the app, even if i let those 27 items download, and restart. With smooth scrolling as above i can rack up tens of thousands of items quite quickly -- presumably because of the rapid redraw requests. Using the cursor keys also racks up download items, but not as quickly. This doesn't happen on the GTK2 version.

I can look into providing more information tomorrow.

@rnorris
Copy link
Collaborator

rnorris commented Jan 17, 2021

Thanks for raising + diagnosing the scroll issues. I will include some form of the attached fix.
I should look to setting my laptop into using Wayland.
It reminds me that I had introduced the 'scroll to zoom' option, such that one could disable it and so a trackpad can be used to pan the map around.
Now with GTK3 one should be able to determine if the scroll event came from a mouse or the trackpad, such that I'd want by default a trackpad scroll to move the map as the scroll direction is in 2D, whereas a scroll from a mouse (i.e. the scroll wheel) would zoom - since most mouse have a single scroll wheel so 1D.
Also should be able then have pinch zooms and possibly swipes to go forward/back through the location list.

Regarding the map download, that is surprising on 2 counts. First the map downloads should only get triggered by the full drawing calculations, there should be little change into how often this occurs. Even if there are more widget draw requests, these should just paint the image to the screen (i.e. not the full drawing calculations). Secondly the recent 'request queue' work should limit number of map requests.

Lastly a further note on the track graph drawing...
Whereas the main viewport has two cairo surfaces to handle the main and mouse location related drawing, the track graphs has a single surface and ATM redraws everything inside the 'draw' signal update. Thus it is incredibly slow for longer/more detailed tracks. I think I'll have to rework it in similar manner so the heavy work is performed outside of the paint draw update cycle.

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 17, 2021

Regarding the map download, that is surprising on 2 counts.

When i get time i'll try to diagnose it. That might not be super quick due to work commitments at the moment.

Lastly a further note on the track graph drawing...

Good to know -- i had already planned some work on track graphs for when GTK3 is settled :) (option to show multiple curves on one graph, so you can directly compare e.g. speed and elevation).

@roooots
Copy link

roooots commented Jan 17, 2021

A provisionally complete version is here:
https://github.com/rnorris/viking/tree/GTK3-WIP

Thanks a lot. I just played with that version a bit. First, I cannot confirm the download issue described by @yourealwaysbe. So far I've tried with Mapnik, OSMAND HD, Bing, DEM. Also, the tiling issue during map downloads described above is gone.

However, I have encountered a severe bug: Viking crashes on attempting to move a trackpoint/routepoint/waypoint: This happens regardless of the type of map displayed, even without map. After selecting a *point, it can still be moved by mouse, but as soon as the left mouse button is released, viking reproducibly CTD. I've used verbose & debug mode, here is some more info:

Upon selecting TP:

** viking [Debug]): vik_trw_layer_create_profile: 0,000000
** viking [Debug]): Key file does not have key “trkpt_selected_statusbar_format” in group “viking”

Upon attempt to move it and release left mouse button:

viking: ../../../../src/cairo.c:524: cairo_destroy: Assertion `CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&cr->ref_count)' failed. Aborted (core dumped)

Aside from that, I've tried a variety of operations on self-created, imported or existing tracks and routes: Splitting, combining, adding trackpoints, reversing, extending. No issues with these so far.

@rnorris
Copy link
Collaborator

rnorris commented Jan 17, 2021

@roooots Oops.
In viktrwlayer.c the function marker_end_move()
simply remove the line with " ui_gc_unref ( t->gc );"
Although this should really be in a #else part for GTK2 usage.

@roooots
Copy link

roooots commented Jan 17, 2021

Yup, that did the trick. I'll do some more testing during the next days and report back if I encounter any issues.

Aside from that - are there any functions etc. you would like to see some testing?

@rnorris
Copy link
Collaborator

rnorris commented Jan 18, 2021

@ yourealwaysbe - The current implementation of gesture zoom is just a proof of concept.
What I'm hoping is you'd be able to find out at the end of a sequence of gesture callbacks what the final scale value is and/or determine if each callback scale value needs to be multiplied together (quite probably judging by this example if many gesture callbacks are made - then it would probably be best to use a g_timeout_add() with a small timeout to wait to see/calculate the final value to use - similar to the window_zoom_timeout() mechanism.

Then from that value, translate to the amount of zoom steps (strictly maintaining the powers of 2) to apply. See what values for 'small', 'medium' and 'large' physical pinch movements give and convert that to zooming in/out 1,2 or 3 levels (or perhaps may be up to 5 levels of change). Without access to a pinching device, I don't know what range the scale value could cover - we'd have to assume the values from your device are typical, as it would be used for all devices.

Hopefully that make sense.

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 18, 2021

@rnorris thanks for the link -- i'll read it later instead of just experimenting.

From my experiments, the scale value in the gesture callback is always relative to the pinch size when the gesture began. I can get it to range from 0 to just over 8 with my fingers. I think this is why the gesture begin callback is useful -- you can save the zoom level of the map at the start of the gesture, and scale the initial saved level on each gesture callback.

If you only allow powers of 2, you can code it to only redraw when it crosses a power of 2 boundary. It's not so nice because it isn't smooth, but there wouldn't be a need for a timeout (unless the user is really flapping their fingers!).

I have some preliminary code that does this. However it moves the map erratically because i haven't quite got it to zoom properly around the focal point of the pinch. I'm working on adapting the existing "zoom in to xy" code for that.

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 19, 2021

edit: diff deleted -- scratch that -- realised a bug that makes it choppier!

@yourealwaysbe
Copy link
Contributor Author

Ok, here's a diff that works ok. I experimented with making it stop moving the map while pinching, but somehow i think it's more natural to be able to move the map and pinch at the same time.

I made a video, using the mouse to show where i was going to pinch. The recording isn't great, but you can still see the choppiness caused by the fixed zoom levels. It's not too bad though.

pinch-zoom.mp4

@yourealwaysbe
Copy link
Contributor Author

yourealwaysbe commented Jan 23, 2021

I've noticed an issue with panning the window -- i think caused by the floating point representation of screen coordinates.

In the video, you can see that there is a bias towards moving to the bottom right. The mouse starts at the crossroads, but drifts as i pan. This is most noticeable when dragging the touchscreen as imperceptible finger movements activate the drift.

bias.mp4

I can eliminate the drift by adding

event->x = (int) event->x;
event->y = (int) event->y;

to draw_mouse_motion in vikwindow.c. However, this kills the pinch to zoom functionality, unless i change draw_mouse_motion to return FALSE. But this doesn't feel like the right solution.

@rnorris
Copy link
Collaborator

rnorris commented Jan 23, 2021

Good find. Yes I could recreate this. Seems to be several faults here :( !

  1. Previously it was modifying the incoming parameter event structure when using gdk_window_get_pointer()
    Not good programming practice IMHO.
  2. Now this is not done in our GTK3 version so the unintended consequence is now the floating point value is passed on in toolbox_move()
  3. Then in vik_window_pan_move() it goes awry using a floating point value in the center screen calculation.
  4. draw_mouse_motion() should be returning FALSE anyway. The “motion-notify-event” signal from the GTK+ ref manual should return a boolean value and so should draw_release() and draw_click() routines too. I was planning to patch that in the main branch already.

Other motion handling event functions aren't sensitive to it, maybe they should round the floating point value, but in general a pixel here or there won't make much difference. Whereas the pan is continually using it to adjust the center and so causing drift.

So ATM I've got:

static void vik_window_pan_move (VikWindow *vw, GdkEventMotion *event)
{
if ( vw->pan_x != -1 ) {
vik_viewport_set_center_screen ( vw->viking_vvp,
vik_viewport_get_width(vw->viking_vvp)/2 - (gint)round(event->x) + vw->pan_x,
vik_viewport_get_height(vw->viking_vvp)/2 - (gint)round(event->y) + vw->pan_y );
...

draw_mouse_motion () :
gint x, y;
#if GTK_CHECK_VERSION (3,0,0)
x = (int)round(event->x);
y = (int)round(event->y);
#else
gdk_window_get_pointer (event->window, &x, &y, NULL);
#endif

toolbox_move(vw->vt, event);

vik_viewport_screen_to_coord ( vw->viking_vvp, x, y, &coord );

@rnorris
Copy link
Collaborator

rnorris commented Feb 7, 2021

Commits tidied and now practically ready to promote (CI build tests pending...) to the main line:

https://github.com/rnorris/viking/tree/GTK3

Further testing would be appreciated.

GTK3 Will now be the default going forwards.
Although GTK2 still is presently available, the next Viking release will be the last to support it and GTK2 bits removed especially if any updates take more advantage of GTK3 available features (e.g. GtkGrids etc...).

@roooots
Copy link

roooots commented Feb 7, 2021

Thanks! Just installed it and will report back if I encounter any issues. If you want any specific stuff tested, let me/us know.

Cheers,
r.

@matthewhague
Copy link

matthewhague commented Feb 7, 2021

Great! Thanks a lot for your work.

I've compiled it and used it a bit. For me, on line 1921 of vikwindow.c, i had to set the value of g_timeout_add to 5 for scrolling via the touchpad to be usable. If it's 10 or above, it's almost like it only draws when i remove my fingers from the touchpad. There are a few erratic redraws every second or two in between.

edit: with a timeout of 5 it's much better than before the timeout, which could be laggy with some tracks to draw.

@matthewhague
Copy link

matthewhague commented Feb 7, 2021

I found another oddity. When viewing the track properties graph, the graph doesn't line up with the mouse marker (small black dot in the centre of the screenshot). It also doesn't line up with the window borders (grey box on the screenshot), which could be a Sway bug, but it's not one i've seen with other applications.

odd-graph

edit: this doesn't seem to be a problem with the GTK3-WIP branch.

editedit: on the screenshot above, not also that only the top part of the graph is showing.

@rnorris
Copy link
Collaborator

rnorris commented Feb 8, 2021

Dang it!
Works OK when using the overall Preferences->Track Properties Dialog->Tabs At Side->[Yes]
But drawing offsets somehow messed up when Tabs At Side->[No] (which is the default).
I'll investigate...

@matthewhague
Copy link

Thanks. It's better with tabs on the side, but still not right for me. I couldn't make the dialog wide enough on my monitor to contain the full length of the graph, and the black dot does not line up with the displayed graph. I suspect this is all the same problem, but it's another data point.

sway-shot

@roooots
Copy link

roooots commented Feb 8, 2021

Um...here's one little bug: With the build based on the February 07 code, the "Scroll to zoom" checkbox behaviour is inverted. That is, scroll to zoom by mousewheel will only work if the checkbox is unchecked :-)

Cheers,
r.

@roooots
Copy link

roooots commented Feb 8, 2021

And one more, though I don't know if it has been present with the GTK2 version already: While drawing a normal track or route can be terminated by double-click, drawing a route via the Route Finder function can't and requires ESC being pressed. I guess it would be more consistent if it could be terminated by double-click as well.

r.

@rnorris
Copy link
Collaborator

rnorris commented Feb 8, 2021

@roooots I can't recreate your "Scroll to zoom" issue.
Regarding the Route Finder, that is an existing issue. I'll make a note to do it, but feel free to create a separate issue so I won't forget.

@roooots
Copy link

roooots commented Feb 9, 2021

That's weird. To make sure it's not some graphical issue with the checkbox, I tried by setting it directly in viking.prefs. The current behaviour here is:

viking.advanced.use_scroll_to_zoom=f: Zoom viewport by mousewheel scrolling is active
viking.advanced.use_scroll_to_zoom=t: Move viewport by mousewheel scrolling is active.

I'm using a "real" mouse on a desktop PC, not a touchpad.

Could anyone else give that a go, please?

@yourealwaysbe
Copy link
Contributor Author

Could anyone else give that a go, please?

I tried with a "real" mouse on my laptop, but got different behaviour! With scroll to zoom checked, the scrollwheel has almost no effect, except occasionally shifting the viewport by a pixel. When scroll to zoom is unchecked, the viewport moves nicely on scroll.

@roooots
Copy link

roooots commented Feb 9, 2021

Muaha, nice. If this is any helpful for debugging, I'm running Ubuntu 20.04 with Xfce 4.14 and compiz 0.9.14.1, libgtk-3-0 is 3.24.20-0ubuntu1.

@yourealwaysbe
Copy link
Contributor Author

Muaha, nice. If this is any helpful for debugging, I'm running Ubuntu 20.04 with Xfce 4.14 and compiz 0.9.14.1, libgtk-3-0 is 3.24.20-0ubuntu1.

I'm on Wayland, so possibly there some diversion in handling there. I can experiment later to see what events are getting fired.

@rnorris
Copy link
Collaborator

rnorris commented Feb 15, 2021

Latest update.

https://github.com/rnorris/viking/tree/GTK3

Fixed the dialog track drawing, which was some hold overs from very old GTK widget size handling <2.14ish so it tried using the values supplied in the configure event (rather than getting from a widget itself). Which made things messy.
Also some differing GTK2/GTK3 resizing criteria/behaviour - which depending on the functions used and on what widget - was a little tricky to get right for all circumstances. Hopefully that is sorted.

Regarding the scrolling, it seems that on Wayland, even mouse scroll wheel events are now of a 'Smooth Direction' (new in GTK3) - so previously it didn't handle this case. It now deconverts such scroll events into the normal Up/Down directions to take appropriate action.

@roooots
Copy link

roooots commented Feb 16, 2021

Thanks.

Both the scroll-to-zoom issue and the right-click-to-terminate feature for creating routes using the Route Finder function appear to be fixed for me know.

However, I stumbled upon a panning issue that obviously was present in previous GTK3 builds already, with the following behaviour, which can best be observed at zoom level 1 pixelfact or lower values (higher magnification). To reproduce, place the mouse pointer on a specific locations on the map, e.g. crossing of two hiking tracks.

-Expected: When panning, the mouse pointer will stay exactly at the same position on map, but the map as a whole will move with the mouse pointer in exactly the direction of mouse movement, that is: moving mouse up = map moves south.
-Observed: When panning, the mouse pointer will move away a bit from its original location on the map. At the same time, the map as a whole still moves with the mouse pointer, but not in precisely the direction the mouse is moving. The displacement of the pointer with respect to the original location will vary with the direction of mouse movement. Example: Pointer placed on track crossing. When moving mouse slightly upwards, two things happen: The pointer will move away from it original map location, heading north-west on the map, at the same time the map won't move straight south either, but somewhat to the east, then somewhat south, etc.

Can anyone reproduce this?
Edit: Ok I see, this appears to be the issue already reported earlier by yourealwaysbe, which I thought had already been fixed. Sorry for the double ;-)

@rnorris
Copy link
Collaborator

rnorris commented Feb 16, 2021

@roooots Indeed I thought I had 'fixed' it. However I still can reproduce the behaviour you describe, so I expect the fix needs further fixing/improvements. Thanks for noticing and reporting.

In vikwindow.c vik_window_pan_move() it should now also perform rounding when setting the new values for vw->pan_x and vw->pan_y from the event->x and event->y values.

vw->pan_x = event->x;
vw->pan_y = event->y;

-->

vw->pan_x = (gint)round(event->x);
vw->pan_y = (gint)round(event->y);

@roooots
Copy link

roooots commented Feb 17, 2021

Awesome - that does the trick. Thanks!

@yourealwaysbe
Copy link
Contributor Author

I really like smooth scrolling and pinch zoom on the trackpad.

I've spotted a weirdness with hiding/showing layers. I have an aggregate layer containing a bunch of external layers. The aggregate layer is set not to show by default, but the external layers inside are all set to show. This means everything should show if i check the box next to the aggregate layer.

However, when i click the checkbox to view the aggregate layer nothing shows. I have to check the box, then select the aggregate layer. Then i have to select one of the internal layers contained. Then all the tracks are drawn.

@rnorris
Copy link
Collaborator

rnorris commented Feb 20, 2021

@yourealwaysbe This works for me on my Debian PC. Do you only see this in the GTK3 version? I'm currently not aware how/why GTK3 would make a difference here. I don't think I have any external layers ready to roll on my Ubuntu machine, so it would take a little more time to set up a test there.

@yourealwaysbe
Copy link
Contributor Author

Panning with the touchscreen is laggy for me. It seems to generate way more events than panning by clicking and dragging on the mousepad. Adding the draw timeout to the redraws seems to fix it. I attach the diff that works for me:
pan-timeout.txt

@rnorris
Copy link
Collaborator

rnorris commented Mar 11, 2021

Thank you @yourealwaysbe and @roooots for the feedback and testing.

The code has been promoted into the mainline as it's hopefully robust enough for general use.

If there are any problems, then they can be addressed individually with new github/sourceforge issues as appropriate.

@rnorris rnorris closed this as completed Mar 11, 2021
@yourealwaysbe
Copy link
Contributor Author

Awesome, thanks a lot for your work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants