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

Formspecs: Fix background elements from interferring with each other #4416

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@rubenwardy
Copy link
Member

commented Aug 7, 2016

Background are defined using background[x,y;w,h;image;clip]
It turns out that clip was stored globally for all backgrounds (as a class member variable) rather than being specific for a particular background.

Before this PR

background[x,y;w,h;image;false]
background[x,y;w,h;image]
background[x,y;w,h;image;true]
background[x,y;w,h;image]

is the same as

background[x,y;w,h;image;true]
background[x,y;w,h;image;true]
background[x,y;w,h;image;true]
background[x,y;w,h;image;true]

as the last background to specify clipping sets the clipping of all the others.

What should've happened

background[x,y;w,h;image;false]
background[x,y;w,h;image;false]
background[x,y;w,h;image;true]
background[x,y;w,h;image;false]

After this PR

the clip only applies to the background it's defined on

@rubenwardy rubenwardy force-pushed the rubenwardy:background branch to a749fbb Aug 7, 2016

@rubenwardy rubenwardy changed the title Fix background formspec elements from interferring with each other Formspecs: Fix background elements from interferring with each other Aug 7, 2016

if (!data->explicit_size)
warningstream<<"invalid use of background without a size[] element"<<std::endl;

if (parts.size() == 4 && is_yes(parts[3])) {

This comment has been minimized.

Copy link
@est31

est31 Aug 20, 2016

Contributor

In order to have better self-documenting code it would be cool to have something more like:

bool clip = false;
if (parts.size() == 4 && is_yes(parts[3])) {
    // etc etc
    clip = true;
}
m_backgrounds.push_back(ImageDrawSpec(name, pos, geom, clip));
@est31

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2016

Otherwise looks good.

@rubenwardy rubenwardy force-pushed the rubenwardy:background branch from a749fbb to d89326b Aug 25, 2016

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2016

Fixed

@est31

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2016

👍

@est31 est31 added the One approval label Aug 25, 2016

@paramat

This comment has been minimized.

Copy link
Member

commented Aug 28, 2016

Author self-approves.

@rubenwardy

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2016

I manage to merge it fine :P

@rubenwardy rubenwardy closed this Aug 29, 2016

@rubenwardy rubenwardy deleted the rubenwardy:background branch Aug 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.