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

Refactor displays, create display widget #186

Closed
toomanybrians opened this issue Sep 23, 2016 · 6 comments
Closed

Refactor displays, create display widget #186

toomanybrians opened this issue Sep 23, 2016 · 6 comments
Assignees
Milestone

Comments

@toomanybrians
Copy link
Member

Now that I have more experience with Kivy, I want to refactor some things.

First, I want to create a widget type called "display" which is used to display the contents of one display on a slide (or on a display via widget targeting). This is very similar to how the dmd and color_dmd widgets work today. So it's sort of renaming those, and then adding the features they have (limited color palettes, tints, dot filters, fps settings) to that widget.

Also let's get rid of the slide_frame widget and just use displays. Cleaner code, and the slide frame is a layer in the widget tree stack that can be removed.

As part of this, clean up some current bugs. For example, some widgets have certain properties that can't be animated because their on_pos() meths pull from their config instead of their live values.

@toomanybrians
Copy link
Member Author

parent_frame will just be replaced by targeting a display with a widget rather than a slide

@qcapen
Copy link
Collaborator

qcapen commented Apr 22, 2017

I think I am up-to-speed enough on how Kivy works to attempt to begin to discuss this issue intelligently :)

I'm trying to understand what the new object hierarchy will look like after this refactoring. Are you advocating getting rid of the current Display object (it is really just a Kivy widget anyway) and replacing it with a Display widget (with an associated ScreenManager that can show slides with transitions, etc.)? DMD and ColorDMD would then be sub-classes of the new Display widget?

I definitely agree that getting rid of the slide_frame widget would simplify things, but we just need to designate which widgets can be slide targets or have child widgets. Container widgets vs. display widgets?

I would also like to add layout widgets based on the various Kivy layouts (box, stack, grid, etc.). These would be useful for auto-adjusting various numbers of widgets on the fly without calculating every coordinate.

I see opportunities to simplify the code and design and do some things more efficiently. Exposing as many properties as possible for animation using Kivy properties (to include rotation and scaling) is a good goal as part of this refactoring as well.

Commence further discussion...

@toomanybrians
Copy link
Member Author

I'm not sure if I'm advocating removing Display object and replacing it with a Display widget. I'm fine with that if that's the case, but that was not what I was getting at.

I'll explain what I was thinking, but with the caveat that I'm fine with other options.. this is just what was in my head at the time.

Currently it's kind of weird because both Display objects and slide frame widgets can be used as "targets" for widgets, and both are types of ScreenManager objects, and both can hold slides, etc. So my thinking was to essentially combine them both. I was thinking they would be called "Displays", but I hadn't really thought about whether they would be instances of ScreenManager or Widget (which I know they are also both the widgets). The idea though is that a Display could also contain a widget which was itself a Display, and that "sub"-Display would have its own slides, each with their own local coordinate system, etc.

My thinking for that was that would be how you'd do a "picture in picture", or something like the 4-up display in WOZ or Hobbit. (Which would be five displays.. the parent plus the 4 screen quadrants.)

There still needs to be some way to map an MPF MC "Display" object to (1) a physical DMD, and (2) the main Window.

Then separate from all this, we have the "DMD" and "Color DMD" widgets. I was thinking those two could go away completely (rather than a subclass of display), because you can still add a display to a slide which is all those widgets were doing anyway. And then the GLSL code for DMDs (dot filter, color reduction, color tinting, etc.) could just be added as options to the general display object. So then if you wanted to put a virtual DMD on a slide, you would add the display widget to that slide (pointing to the Display object that will be its source) and enable the dot filter settings.

All that said, maybe we map this closer to kivy and just call it a "slide manager" or "slide container" or something like that, instead of "display" which is very confusing for people?

I also really like the idea of adding layout widgets in whatever way you think makes the most sense?

As you know, now is the time to rip this whole thing apart and redo it (if we need too). I was very much learning Kivy as I was going along and there are probably better ways to do a lot of things here.

@qcapen
Copy link
Collaborator

qcapen commented Apr 25, 2017

I think the overall design of the graphical part of the MC is pretty good and mirrors Kivy's architecture quite well. I don't think we'll need any major overhauls in this effort. Most of the things I have found in the code that could be improved are minor consolidations and efficiency gains, not big design changes.

After prototyping a few things and doing some more code inspection, I do not recommend merging the display object and widgets into one. Here is what I think it should look like (basically what you outlined above):

Display objects are conceptually the same as before. They are specified in the configs by name and are static (can't create or destroy display objects at run-time). These become targets for slides and widgets. This class will essentially become the existing Display class merged with SlideFrame (derived from ScreenManager).

Slides do not change at all, except that display objects are now the only targets for slides.

Display widgets will be just like the existing "DMD" and "Color DMD" widgets (which can be eliminated). They get their graphical source from a Display object, but can further manipulate the image using effects which can be specified in the config (dot filter, color reduction, etc.). I am considering allowing effects to be applied to any widget (like animations). We can easily provide a library of built-in effects and a simple interface to roll your own (for those who are adventurous enough to try glsl). I tried to come up with a different name for either the widget or object so we don't have two different things called the same name, but I'm not happy with any other names yet (DisplayOutput, DisplayMirror, DisplayCopy are my favorite alternate names for the widget so far).

I think that is all that is needed for this refactoring. The code becomes simpler and the migration tasks should be pretty straightforward to migrate.

A couple of miscellaneous thoughts:

  • Should we prevent display recursion (what happens if I create a picture in picture using display widgets and select the same display source for both)?
  • Would it be more efficient to have the display object render to a framebuffer and use that framebuffer in the display widget rather then swapping widget parents? Not sure if a parent swap causes the entire child branch to be re-rendered or not. Answering this will require some prototyping and further investigation.
  • Thinking about the layout widgets: layout widgets can contain other widgets, but how should they be added? Layout widgets become a valid widget target? They should be dynamic (add and remove widgets at runtime) or else there is really not much advantage to using them. Slides can be added to layouts by adding display widgets to the layout. Also, size and position can be calculated for widgets that are added to layouts, but the size and position hints become important, especially if widgets contained in the layout will be different sizes. Will continue to brainstorm about layout widgets. Please feel free to add your thoughts...

@qcapen qcapen self-assigned this Apr 25, 2017
@qcapen qcapen added this to the 0.50 milestone Apr 25, 2017
@qcapen
Copy link
Collaborator

qcapen commented Jun 1, 2017

Display refactor changes needing migration:

  • 'dmd' widget now replaced by 'display' widget with new 'effects' parameter ('type: dmd').
  • 'color_dmd' widget now replaced by 'display' widget
  • The following dmd widget settings are now moved to the effects section ('type: dmd'): dot_filter, blur, pixel_size (now dot_size), pixel_color (now dot_color), dark_color (now filter_color), bg_color (now background_color, gain, shades, luminosity
  • The following color_dmd widget settings are now moved to the effects section ('type: color_dmd'): dot_filter, blur, pixel_size (now dot_size), dark_color (now filter_color), bg_color (now background_color, gain, shades
  • 'slide_frames' widget is replaced by 'display' widget AND a corresponding display entry in the 'displays:' config section must be created (the name: setting becomes the new display name). 'width' and 'height' are used for both the new widget and the new display.
  • 'image' widget 'loops' property now mirrors the other looping settings in MPF (-1 to loop infinitely, 0 no repeats/loops, > 0 the number of times to repeat after the first time through). Can just subtract 1 from any existing image:loops setting.
  • 'animations' now use the widget anchor position when animating position values (x, y, pos) rather than the lower left corner. The anchor offset position for each animated widget must be calculated and existing position animation values should be adjusted accordingly so that animations appear the same as in config_file = 4.
  • Rename config section 'physical_dmds' to 'dmds'
  • Rename config section 'physical_rgb_dmds' to 'rgb_dmds'

@qcapen
Copy link
Collaborator

qcapen commented Jun 1, 2017

Implemented in 0.50.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants