search

sanette / bogue

GUI library for ocaml based on SDL2
http://sanette.github.io/bogue/Principles.html
Other
181 stars 15 forks source link

Getting Bogue to work with current TSDL #3

Closed guygastineau closed 2 years ago

guygastineau commented 3 years ago

Hi, your project is really cool ;)

I just had an issue where OPAM couldn't install TSDL with a low enough version to work with Bogue. I cloned your repo, and I got errors about variants not supporting tags between Sdl.enum and B_trigger.sdl_event I fixed it by adding the variants `Display_event and `Sensor_update to the sdl_event type in the B_trigger. I don't know if that is a good way to do things, but it is what I had to do to get bogue installedwith OPAM.

I am happy to submit a PR if you want it. Perhaps there are other concerns with bumping the tsdl dep version? Just let me know. I would have already opened a PR, but my emacs config deletes all trailing whitespace on save (and I always forget ;) ), so I will need to redo the patch to avoid littering a PR with trailing whitespace removals. On that note, are you interested in a PR that cleans all of the trailing whitespace out of the manually edited files? Otherwise, I have to remember to remove it from the files all at once, so I can drop the commit when submitting work to a project. I won't get bent out of shape either way, but I figured I should ask at least ;)

PS. Sorry I didn't post any dumps of the errors I got, but I assume you know about them; I assumed that is why there is an upper bounds placed on the tsdl dependency.

UPDATE: I get the following error when trying to execute the minimal example as well as any example with the boguex tool:

% ocamlfind ocamlopt -package bogue -linkpkg -o minimal -thread minimal.ml
% ./minimal
cp: cannot stat '/home/guy/.opam/bogue/lib/bogue/../../share/bogue/themes.tgz': No such file or directory
Fatal error: exception Not_found

I have a themes directory where the program expects a tar gzip archive. The only change I made other than adding the two variants was removing the and and upper bound on the tsdl version in dune-project. Perhaps someone here knows how to help with this problem.

sanette commented 3 years ago

Hi, thanks for your message (and: Happy New Year!) yes, I know there are compatibility issues, these are not easy to automatically solve, see: https://github.com/ocaml/opam-repository/issues/16741 Did you read the install file http://sanette.github.io/bogue/INSTALL.html ? The suggestion is to install tsdl.0.9.6

One the one hand, I would like to keep bogue compatible with the SDL2 version shipped by default by the latest LTS ubuntu. On the other hand, tsdl implements a higher version of SDL which is unfortunately not backward compatible.

I have to admit bogue was released with ubuntu LTS 18.40 and I didn't really check the new SDL shipped with Ubuntu 20.04 (because for me the combination ubuntu 20.04 + tsdl.0.9.6 still works nicely). EDIT: if you can make it work simply by using tsdl.0.9.6 then I think I prefer to continue supporting ubuntu 18.04 for a couple of years more. Of course, there is no problem in eliminating trailing whitespace ;)

Concerning the last issue in your post concerning theme.tgz, I have to look. What is your OS? Did you do 'opam install .' ? The theme.tgz is supposed to be unpacked when installing bogue.

guygastineau commented 3 years ago

Oh, I see, so there is more holding back migration to higher tsdl versions than some polymorphic variants ;) (Happy new year to you too!)

Hmm, yeah I did see that suggestion in NSTALL.md. After several failing attempts to get Bogue to install through OPAM I realized it was the downgraded tsdl package that was breaking during installation. If I install the newest version it is fine.

I am on up to date Arch Linux, so my my libraries are often very recent. My SDL is version 1.2.15-14 and my SDL2 is version 2.0.14-1. It seems like SDL2 (that is what your library uses right?) ranges from 2.0.8 (in 18.x) to 2.0.12 in the latest Ubuntu found here. Is the breaking change in SDL between 2.0.8/12 and 2.0.14? TBH, I have never used SDL for anything before. All of my GUI crafting has been with GTK and libraries on top of it.

Currently for expedience at work I make a couple GUI clients with the gui tools built into Racket. That is fast and useful, but the support for colors and theming is TERRIBLE. Also, I love Scheme, but ML's hold the secret key to my heart. I am also trying out Haskell's fi-gtk-declarative. It is a pretty neat project, but I like the simple flexibility it appears your project provides. Also, even though I miss some Haskell features while programming in OCaml I have found that OPAM and Dune (Yay SEXP) are so much better than Cabal. All that is to say that I am very interested in getting Bogue to work on my machine ;)

So, a couple things. For now I am definitely not interested in downgrading my SDL2 package on Arch, but I don't mind maintaining the fixes for forward compatibility until Ubuntu upgrades its packages. A plus is that we will have some easy to merge patches for when that happens ;)

So we are left with the weird missing themes.tgz file. Well, I looked through the dune stuff, and I see that it puts everything in the themes directory, but I don't see anywhere that it expects a .tgz. Does your Bogue setup use an archive instead of a plain directory for themes? If so, maybe the way Bogue opens themes through TSDL means that behind the scenes TSDL expects a path to be archived with the new incompatible version? I guess that makes sense as the point of origin for this problem. I will look through lib/b_theme.ml and see if I can trace the problem back through tsdl's api ;)

EDIT: I just reread your comment about it. Yes, I opam install .'d, and I have the proper contents in the themes directory of my installation. I am thinking it must be an incompatibility with upstream TSDL as I said above.

Haha, I have literally been in an (almost) nasty fight about trailing whitespace before, so I am cautious about it now. lol

I have some stuff to do this weekend to ramp up for work on Monday (lots of Rust and some Scheme), so I doubt I will have the themes.tgz issue solved until sometime next week, but I will gladly submit a PR that simply removes all trailing whitespace. I definitely wouldn't want to clutter small PRs with potentially hundreds or thousands of unrelated whitespace eliminations.

Thank you very much for your quick response and open mindset <3

guygastineau commented 3 years ago

BTW, I think I found the code that expects the themes.tgz file:

https://github.com/sanette/bogue/blob/4ef9e50eb42b540b6719dc65ee207349b5218f00/lib/b_theme.ml#L225

Since opam install . from the project doesn't use an archive this seems weird to me. I am going to try simply copying the contents to ~/.config/bogue/, and I expect it to work.

UPDATE!!!!! So, I just tried running the minimal example, and IT WORKED!!!! ;) The weird thing is that, looking at the code, I cannot figure out how Bogue can expect a themes.tgz file to exist since the dune files don't specify one anywhere that I have found. Do you perhaps used to use one? Since it doesn't look for it if ~/.config/bogue/themes exists I imagine it would be possible for the change to have happened without any longtime users (or the creator) having realized? IDK, I am just tryiong to figure it out, so we can make it so this roadblock doesn't come up for anyone else.

sanette commented 3 years ago

yeah, in principle theme.tgz is not used anymore, it remains there for older versions, that's why I'm a bit surprised it pops up now, because in principle doing opam install . should install the theme directory

sanette commented 3 years ago

Oh now I remember, there is a TODO line 205 which precisely says I have to update the new location of the share dir... hehe (sorry it's in French)

guygastineau commented 3 years ago

No problem. I am glad that my issue brought it to mind again.

So, opam install . put the themes contents in ~/.opam/bogue/share/bogue/themes/, but it didn't put it in ~/.config/bogue/themes/. I had to do that manually. You are saying that opam install . should have put the contents in ~/.config/bogue/themes/?

sanette commented 3 years ago

no, I'm just saying that Bogue should look for the theme in ~/.opam/bogue/share/bogue/themes/ in case ~/.config/bogue/themes/ does not exist

sanette commented 3 years ago

no, I'm just saying that Bogue should look for the theme in ~/.opam/bogue/share/bogue/themes/ in case ~/.config/bogue/themes/ does not exist

I've just uploaded a new version which should fix this. Could you try?

guygastineau commented 3 years ago

Hi, the changes work (I briefly had a message on here claiming it didn't because I forgot to recompile the test project - it was very confusing ;) )

guygastineau commented 3 years ago

It seems like you already removed the trailing whitespaces. I am blown away by your courtesy.

I will make a fork before I go to sleep tonight, and I will push a branch with my patch to use the latest tsdl. The patch isn't very big, but still I hope it will help when the time comes to transition your upstream work to support the latest sdl.

You have some interesting work here, so I will keep playing around with it to learn how to use it better. I have encountered a couple glitches like gaps in the rendering for border-radius affected buttons. I am not sure if that is normal or if it is because of using it with the next tsdl. Hopefully, we can iron things out together. I would include real screenshots in isolated issues for such bugs ;)

sanette commented 3 years ago

I have encountered a couple glitches like gaps in the rendering for border-radius affected buttons.

I noticed this too :( This started to happen when SDL2 corrected a long-standing bug where end-points of lines were not correctly rendered. It still have to correct my circle algorithm accordingly :(

EDIT: it's worse than I imagined, SDL introduced a new bug (and a bad one!), see https://discourse.libsdl.org/t/sdl-renderdrawline-endpoint-inconsistency/22065/8

sanette commented 3 years ago

I have encountered a couple glitches like gaps in the rendering for border-radius affected buttons

this should be fixed now

guygastineau commented 3 years ago

Thanks. I really have to crunch on some work stuff, but I might have time to check it out later today. I assume you tested it on the version of SDL2 you have, so when I test it I will be able to verify that it works on the latest SDL2 too :)

EDIT: I haven't forgotten about testing it with bleeding edge everything :upside_down_face:

sanette commented 2 years ago

The original issue is solved by the new opam version 20210917 Feel free to re-open if it doesn't work for you