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

Make height and width inferrable for (abstract) CairoSurface #338

Merged
merged 1 commit into from
Jan 1, 2021

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 31, 2020

Gtk.jl and other libraries have containers with fields that are
typed abstractly as ::CairoSurface. This allows width and
height to be inferrable even for the abstract type.
It enforces an "interface" expected of all CairoSurface subtypes.

Gtk.jl and other libraries have containers with fields that are
typed abstractly as `::CairoSurface`. This allows `width` and
`height` to be inferrable even for the abstract type.
It enforces an "interface" expected of all CairoSurface subtypes.
@timholy
Copy link
Member Author

timholy commented Dec 31, 2020

@lobingera, if you're fine with saying goodbye to appveyor, don't forget to delete the webhooks (Settings -> web hooks).

@lobingera lobingera merged commit ea824ad into master Jan 1, 2021
@lobingera
Copy link
Contributor

LGTM (although i wonder what the usecase is ...), web-hooks to appveyor deleted.

@timholy timholy deleted the teh/inferrability branch January 2, 2021 10:53
@timholy
Copy link
Member Author

timholy commented Jan 2, 2021

although i wonder what the usecase is

Marginally better performance with Gtk (since Gtk doesn't store which kind of CairoSurface it is), but the real benefit is better precompilability and thus lower latency on first use. See JuliaLang/www.julialang.org#1111 and a preview at https://julialang.netlify.app/previews/pr1111/blog/2021/01/precompile_tutorial/.

This is a very small change and thus it has only small impact (you wouldn't notice anything from this change by itself), but it's one of a long list from tracking down sources of inference failures in my soon-to-be-released GtkObservables package.

timholy added a commit that referenced this pull request Jan 2, 2021
@timholy
Copy link
Member Author

timholy commented Jan 2, 2021

Oops, though I see I messed up this test...fixed in #339.

@lobingera
Copy link
Contributor

I see. But; as far as i understand, the width and height stored in the surface type are more informative and the dimensions of create surfaces - not necessariiy how the data is arranged in memory (see stride information in https://www.cairographics.org/manual/cairo-Image-Surfaces.html). And your comment about "Gtk doesn't store which kind of CairoSurface it is" - i don't think Gtk offers more than the CairoContext in case of widget (re-)drawing and afair in cases of redrawing you get clipping, but not more information. So deriving width+height from Context -> Surface for rendering on screen (and your definitions for precomp for user_to_device is another indication...) might work in some cases, but not in the general case with e.g. overlapping windows.

lobingera pushed a commit that referenced this pull request Jan 2, 2021
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