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

[builtins] adds ToggleRegionVisualization builtin #7932

Merged
merged 2 commits into from Sep 2, 2015

Conversation

mkortstiege
Copy link
Member

Adds new ToggleDirtyRegions builtin. This enables on-the-fly dirty-region visualization toggling without the need to restart Kodi.

Requested by @HitcherUK.

@mkortstiege mkortstiege added Type: Feature non-breaking change which adds functionality Type: Improvement non-breaking change which improves existing functionality labels Aug 31, 2015
@mkortstiege mkortstiege added this to the Jarvis 16.0-alpha3 milestone Aug 31, 2015
@Hitcher
Copy link
Contributor

Hitcher commented Aug 31, 2015

Thanks.

@mkortstiege
Copy link
Member Author

@Montellese objections?

@mkortstiege
Copy link
Member Author

Will shove this in later today if there are no objections.

@Montellese
Copy link
Member

Could you rename the builtin to make it clear that it toggles the visualization? ToggleDirtyRegions sounds to me like it switches between two different modes of dirty regions (even though there are at least three).

@mkortstiege
Copy link
Member Author

Sure. What about ToggleDirtyRegionVisualization or just ToggleDRVisualization? A bit long maybe ..

EDIT: ToggleRegionVisualization would work for me as well ;)

@mkortstiege mkortstiege changed the title [builtins] adds ToggleDirtyRegions builtin [builtins] adds ToggleRegionVisualization builtin Sep 2, 2015
@mkortstiege
Copy link
Member Author

Made it ToggleRegionVisualization now.

@Montellese
Copy link
Member

I'd go with ToggleDirtyRegionVisualization as it explains best what it does. ToggleRegionVisualization is not that obvious by itself without any documentation.

@topfs2
Copy link
Contributor

topfs2 commented Sep 2, 2015

Im guessing that the original advanced settings is because not all visualisations understand dirty regions? Kind of outside this PR but wouldn't it be better if they simply flagged their support in the add-on.XML, removing the need for this?

@mkortstiege
Copy link
Member Author

@Montellese. Will do.
@topfs2 it's not about visualizations ;) This is is to toggle dirty region visualization on/off without the need to restart Kodi.

@topfs2
Copy link
Contributor

topfs2 commented Sep 2, 2015

Oh wait. Is this the debug were it displays them on screen?

@topfs2
Copy link
Contributor

topfs2 commented Sep 2, 2015

Do we name debug stuff like this in any consistent matter. Would be nice if you instantly knew it is debug related. Perhaps something like DebugShowDirtyRegions

@mkortstiege
Copy link
Member Author

Updated. Builtin now uses ToggleDirtyRegionVisualization. I've also added a keyboard keymap commit that enables this via ctrl+shift+r (analogue to ToggleDebug).

@topfs2, I'd like to keep the term Toggle in there without making it longer.

@Montellese
Copy link
Member

jenkins build and merge

@topfs2
Copy link
Contributor

topfs2 commented Sep 2, 2015

@mkortstiege yeah probably good with toggle naming tbh

@jenkins4kodi jenkins4kodi merged commit ceb0675 into xbmc:master Sep 2, 2015
@mkortstiege mkortstiege deleted the toggle-dr-builtin branch October 6, 2015 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants