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

Import missing functions from Graphics. #254

Merged

Conversation

GunnarFarneback
Copy link
Contributor

In #216, importall Graphics was converted to a list of more specific imports, since importall was being deprecated. That list is not complete, however, as can be witnessed by

julia> import Cairo, Graphics

julia> filter(x -> getfield(Graphics, x) != getfield(Cairo, x),
              intersect(names(Graphics), names(Cairo)))
8-element Array{Symbol,1}:
 :clip_preserve              
 :device_to_user!            
 :device_to_user_distance!   
 :rel_move_to                
 :stroke_transformed         
 :stroke_transformed_preserve
 :user_to_device!            
 :user_to_device_distance!   
julia> import Cairo, Graphics

One effect of this is that text function

function text(ctx::CairoContext, x::Real, y::Real, str::AbstractString;
              halign::AbstractString = "left", valign::AbstractString = "bottom", angle::Real = 0, markup::Bool=false)
    move_to(ctx, x, y)
    save(ctx)
    reset_transform(ctx)
    rotate(ctx, -angle*pi/180.)

    set_text(ctx, str, markup)
    update_layout(ctx)

    extents = get_layout_size(ctx)
    dxrel = -align2offset(halign)
    dyrel = align2offset(valign)
    rel_move_to(ctx, dxrel*extents[1], -dyrel*extents[2])

    show_layout(ctx)
    restore(ctx)
    w, h = Graphics.device_to_user(ctx, extents[1], extents[2])
    BoundingBox(x+dxrel*w, x+(dxrel+1)*w, y-dyrel*h, y+(1-dyrel)*h)
end

computes a bogus bounding box when device and user coordinates are not the same. Graphics.device_to_user is a thin wrapper that repackages the data and calls device_to_user!. This is supposed to dispatch back to Cairo but since Cairo.device_to_user! is currently a separate function from Graphics.device_to_user!, instead the do nothing fallback definition in Graphics is used and coordinates are not transformed.

This PR adds all those missing functions to the import list.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 96.481% when pulling 902f18d on GunnarFarneback:import_all_from_Graphics into 25990f1 on JuliaGraphics:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 96.481% when pulling 902f18d on GunnarFarneback:import_all_from_Graphics into 25990f1 on JuliaGraphics:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 96.481% when pulling 902f18d on GunnarFarneback:import_all_from_Graphics into 25990f1 on JuliaGraphics:master.

@lobingera
Copy link
Contributor

This could even solve #223 which i tracked down a problem with the bounding Box also.

@timholy
Copy link
Member

timholy commented Sep 2, 2018

This might also allow me to revert JuliaGizmos/GtkReactive.jl#68. Great detective work @GunnarFarneback!

We should merge and tag this ASAP. Any objections @lobingera?

@tknopp
Copy link
Contributor

tknopp commented Sep 2, 2018

@timholy: Please! The only objection has Tony since my fix of the Homebrew issue is not as clean as the right fix (which only someone can do having insights into Homebrew).

@timholy
Copy link
Member

timholy commented Sep 2, 2018

Tony is of course right, but nevertheless we should proceed. Any potential problems caused by the bandaid seem likely to be minor, whereas the bandaid fixes major problems. And once this merges it too will fix major problems. Two majors outweigh a minor; we cannot let the perfect be the enemy of the good.

While I could go ahead and do it myself I'd still rather give @lobingera a chance to weigh in, so I will keep that itchy trigger finger in check for a little while at least.

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.

5 participants