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

Perform unit conversions manually from cairomatrix #68

Merged
merged 2 commits into from
Jun 14, 2018

Conversation

timholy
Copy link
Member

@timholy timholy commented Jun 14, 2018

This fixes a strange bug I've seen in remote X sessions, which I've finally gotten around to tracking down: it's a failure to correctly convert between device and user units. For reasons I don't understand, Cairo's cairo_device_to_user fails for me across a remote X session, despite the fact that when I query the current transformation matrix with get_matrix it seems fine. Therefore the workaround is to get the matrix and do the mathematics myself. Weird.

I suspect this fixes #67.

CC @lobingera.

@timholy
Copy link
Member Author

timholy commented Jun 14, 2018

Test failure due to JuliaGizmos/Reactive.jl#178 (comment) Nvm, it was just internal representation changes in Reactive, easily fixed.

@lobingera
Copy link

lobingera commented Jun 14, 2018

@timholy What is your definition of 'fails for me across a remote X session' and who is the owner of the cr?
I just looked into libcairo source, device_to_user is backend specific, but the default is:

void
_cairo_gstate_device_to_user (cairo_gstate_t *gstate, double *x, double *y)
{
    cairo_matrix_transform_point (&gstate->ctm_inverse, x, y);
}

so it's assumed a correct inverse is existing. Which might be not true on all devices.

@timholy
Copy link
Member Author

timholy commented Jun 14, 2018

At a "high level" (what the user experiences, not the actual code issue) I've seen it in several applications:

  • in ImageView, zooming works fine when I run it on my local machine, but on a remote machine the selected region doesn't match the region it zooms in on
  • in ProfileView, the "hover" (displaying the line of code corresponding to a particular box) works locally but fails remotely
  • In GtkReactive/examples/drawing.jl, no lines appear (but debugging shows it's not a failure to detect mouse events)

I used the last one to track this down by inserting a @show btn here. On my local machine, the location of the button click would be reported in normalized coordinates (the range [0,1]) because of this line. Remotely, the location of the button click was reported in pixels. Surprisingly, though, if I inserted a @show get_matrix(getgc(c)) here then it revealed that Cairo knew what the transformation was, it just seemed not to be applying it.

@lobingera
Copy link

Cairo knew what the transformation was, it just seemed not to be applying it.

Cairo knows relative to surface, but not necessarily to how this is mapped (in the X11 sense of map).

@timholy
Copy link
Member Author

timholy commented Jun 14, 2018

It's just weird because you can look at the inputs and outputs of cairo_device_to_user and it's clearly performing an identity transformation (i.e., the values are unchanged), yet get_matrix returns something like CairoMatrix(200, 0, 0, 200, 0, 0) (and it's the latter that indicates the correct behavior).

@lobingera
Copy link

See again the c code above: for device_to_user, the ctm_inverse is used not the ctm. It's an obligation of the backend to keep them connected, but ... And btw: i don't think that's used very often, so this might be actually a bug in cairo (if reproducible).

@codecov-io
Copy link

Codecov Report

Merging #68 into master will increase coverage by 0.28%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage   92.12%   92.41%   +0.28%     
==========================================
  Files           6        6              
  Lines         851      857       +6     
==========================================
+ Hits          784      792       +8     
+ Misses         67       65       -2
Impacted Files Coverage Δ
src/graphics_interaction.jl 94.44% <100%> (+0.15%) ⬆️
src/widgets.jl 91.35% <0%> (+0.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8179c20...038e563. Read the comment docs.

@timholy timholy merged commit c1ea14b into master Jun 14, 2018
@timholy timholy deleted the teh/unit_conversion branch June 14, 2018 18:00
@GunnarFarneback
Copy link
Contributor

Very possibly JuliaGraphics/Cairo.jl#254 is the root cause, although remote X shouldn't have anything to do with it.

@timholy
Copy link
Member Author

timholy commented Sep 2, 2018

Agreed that remote X must have been a red herring. My laptop has a high-resolution display with lots of pixels; I suspect now that ImageView was using 1:1 scaling for my local display of images, and thus it circumvented the problem. The "monitor" provided by x2go is smaller than many of the images I work with, and thus the bug was revealed.

@timholy
Copy link
Member Author

timholy commented Sep 20, 2018

Yes, this was indeed caused by JuliaGraphics/Cairo.jl#254, and can now be reverted. See #78.

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.

Machine-specific test failures
4 participants