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

Get package working on 0.7/1.0 #99

Merged
merged 14 commits into from
Sep 3, 2018
Merged

Get package working on 0.7/1.0 #99

merged 14 commits into from
Sep 3, 2018

Conversation

timholy
Copy link
Owner

@timholy timholy commented Aug 30, 2018

This requires JuliaGizmos/GtkReactive.jl#76 but is, I think, otherwise ready. CC @GunnarFarneback.

@GunnarFarneback
Copy link

The conditional loading looks cleaner than it did before, very nice. Tests pass for me locally both without and with GtkReactive#76. Actually running ProfileView without produces just the bars, but no annotations. With I do get annotations, of sorts, but the experience can at best be described as weird. I'll check if it's the same on another computer tomorrow.

It looks like .travis.yml needs some using Pkg for 1.0.

@timholy
Copy link
Owner Author

timholy commented Aug 30, 2018

I've got the Travis script update already done locally, I was just waiting for the tag to be merged to METADATA.

But it's a little concerning that you're not seeing annotations. I do see them, and in fact the image in the new README was collected on the new version. Have you updated to the latest GtkReactive and then added GtkReactive#76? The reason I ask is because I suspect that JuliaGizmos/GtkReactive.jl#73 was an insanely-important commit (and the only part that mattered was to delete a single line, yield(), from draw). That took me more than one whole day to track down, and since I added it myself (JuliaGizmos/GtkReactive.jl#2) it was definitely a "kick-me" moment. I think some change in Julia's task-handling exposed the problem, but it was latent for a long time.

@GunnarFarneback
Copy link

I believe I should have the right code after doing

pkg> add GtkReactive#master
pkg> add ProfileView#teh/julia0.7

With released GtkReactive (i.e. without PR 76) I had no annotations at all. With master I get the annotation shown as I move the mouse over it, but with a lot of pixels tailing around. After moving the mouse around for a bit it can look like this:
profileview
But as I said I'd like to check with some more computer. This one isn't entirely at its best, so it could be a local problem.

@timholy
Copy link
Owner Author

timholy commented Aug 30, 2018

Hmm, I have seen that. But I'm not getting it currently since that fix.

I am not sure (I'm still getting used to the new Pkg), but I think there's a chance that add GtkReactive#master doesn't give you the latest if you already have a master branch that you checked out earlier. Not sure though. If you use dev you can inspect the git log and see if it matches the one on GitHub. (If not, do a git pull to update.)

My guess, though, is that you are working with the latest and that we have a problem somewhere.

@GunnarFarneback
Copy link

Yes, it's the same with dev and a freshly pulled master.

@timholy
Copy link
Owner Author

timholy commented Aug 30, 2018

That's not good. Perhaps insert some @async @show statements in this block and see if you can figure out what's going wrong? My guess is there's some kind of mismatch between the coordinates of the erase and the coordinates of the draw, so variables like lasttextbb, r, xu, and yu seem particularly important. If you're putting it in an @async (which you need to, since you can't yield from a callback), you might want to have all them end up in the same @async otherwise timing might be confusing.

@GunnarFarneback
Copy link

Looked a bit into that but will have to wait until tomorrow to try to understand it. However, looking at what's going on on the screen I can't help getting the impression that it's erasing a flipped box, leaving the text both left and right of the center but erasing above and below in the center.

@GunnarFarneback
Copy link

Problem is fixed for me with JuliaGraphics/Cairo.jl#254.

@timholy
Copy link
Owner Author

timholy commented Sep 2, 2018

We can merge and tag this any time now. But if @lobingera agrees that we can tag a new Cairo release, I'd kind of like to make it dependent on the fixed Cairo. So I'll hold off for a little while anyway.

@GunnarFarneback
Copy link

Maybe bump Cairo lower bound to 0.5.6?

@timholy timholy merged commit 44eee47 into master Sep 3, 2018
@timholy timholy deleted the teh/julia0.7 branch September 3, 2018 14:39
@timholy
Copy link
Owner Author

timholy commented Sep 3, 2018

Thanks for all your help, @GunnarFarneback, it's been a pleasure!

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

Successfully merging this pull request may close these issues.

2 participants