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

Add lazy rendering support to Layout #809

Merged
merged 4 commits into from Sep 16, 2021

Conversation

nebbishhacker
Copy link

Demo here: https://www.espruino.com/ide/?gist=cc5f26ad897ecaa531b698781d47964a

The overhead of the lazy rendering logic seems overall comparable to the amount of time it saves by not re-rendering things. In the demo most of the time is spent calling layout.update().

The one thing this change doesn't currently handle is background colors on "h" or "v" layoutObjects. Those are kind of tricky, since their children are drawn on top of them.

@gfwilliams
Copy link
Member

Thanks! This looks really clean.

Using getModified is a nice trick but I think it'll backfire on the Pixl and Bangle.js 2 where getModified is used to know how much of the LCD to 'flip', so we might have to avoid that.

You've probably thought about this more than me, but to me it seems like we probably don't need to worry about background colours in h and v... Maybe just during the initial construction phase we just push 'bgCol' down the tree to the children of the group that don't have their own bgCol defined?

@nebbishhacker
Copy link
Author

Ah, good point in regards to getModified, I guess I'll have to change that to record the x/y/w/h of the object instead. (Unless you want to add something along the lines of a g.setModified() method that we could re-mark an area as modified.)

@nebbishhacker
Copy link
Author

Maybe just during the initial construction phase we just push 'bgCol' down the tree to the children of the group that don't have their own bgCol defined?

Parent bgCols would still need to be handled by the lazy rendering code, since there might be areas affected by the background color that aren't part of any children.

To illustrate, currently this layout:

var layout = new Layout( {
  type:"v", bgCol:"#f00", c: [
    {type:"txt", font:"20%", bgCol:"#0f0", label:"12:00", id:"time" },
    {type:"txt", font:"6x8", bgCol:"#0f0", label:"The Date", id:"date" }
  ]
});

produces this image:

screenshot (15)

If I enable lazy rendering, it instead results in this:

screenshot (16)

Note how the red areas that aren't covered by any children are missed completely.

@nebbishhacker
Copy link
Author

Okay, I think I've got bg colors working correctly. Demo: https://www.espruino.com/ide/?gist=c9e3ecbea2dfb149314c0130b7698665

@gfwilliams
Copy link
Member

Looks nice, thanks! It does make the apps nice and tidy.

I just did a quick time test on this running on a Bangle2 and it's 84-180ms for just the lazy rendering though, when literally just re-rendering everything using the old code is 60ms. Minification would help I guess (when I figure out why the library breaks when minified).

Having said that, the update portion of the code takes 120ms as well, so that's hardly quick - but as I'd said in the forum post I thought in a lot of cases it might be possible to avoid calling that for every render.

So I guess it reduces flicker on the Bangle 1, but I guess this wouldn't be something we did for performance unless we actually build it into the Espruino firmware as native C code.

@nebbishhacker
Copy link
Author

84-180ms for just the lazy rendering though, when literally just re-rendering everything using the old code is 60ms.

That's a larger difference than I was hoping for... let me do some benchmarking on my Bangle 1.

For these benchmarks Layout is being executed from storage, with pretokenization enabled.

Benchmark code:

var Layout = require("Layout");

for (let lazy in [false, true]) {
  var layout = new Layout(
    {type:"v", bgCol:g.getBgColor(), c: [
      {type:"h", c: [
        {type:"txt", font:"20%", label:"12:00", id:"time" },
        {type:"txt", font:"10%", label:"00", id:"seconds" }
      ]},
      {type:"h", c: [
        {type:"txt", font:"6x8", label:"Some text", id:"foo" },
        {width: 20},
        {type:"txt", font:"6x8", label:"2020-01-01", id:"date" }
      ]},
    ]
  }, null, {lazy: lazy});

  g.clear();

  let t;

  console.log(`(lazy:${lazy}) Initial render:`);
  t = getTime();
  layout.render();
  console.log((getTime() - t) * 1000);

  console.log(`(lazy:${lazy}) No changes:`);
  t = getTime();
  layout.render();
  console.log((getTime() - t) * 1000);

  console.log(`(lazy:${lazy}) One change:`);
  layout.seconds.label = "30";
  t = getTime();
  layout.render();
  console.log((getTime() - t) * 1000);

  console.log(`(lazy:${lazy}) All changed:`);
  layout.time.label = "10:30";
  layout.seconds.label = "45";
  layout.date.label = "2021-09-15";
  layout.foo.label = "Other Text";
  t = getTime();
  layout.render();
  console.log((getTime() - t) * 1000);
}

Commit 74e739d, with bg color handling:

(lazy:0) Initial render:
70.64819335937
(lazy:0) No changes:
72.54028320312
(lazy:0) One change:
72.23510742187
(lazy:0) All changed:
72.69287109375
(lazy:1) Initial render:
107.11669921875
(lazy:1) No changes:
56.82373046875
(lazy:1) One change:
76.56860351562
(lazy:1) All changed:
140.56396484375

Commit 6bd606b, before bg color handling:

(lazy:0) Initial render:
70.28198242187
(lazy:0) No changes:
72.265625
(lazy:0) One change:
72.11303710937
(lazy:0) All changed:
72.3876953125
(lazy:1) Initial render:
88.10424804687
(lazy:1) No changes:
40.00854492187
(lazy:1) One change:
51.48315429687
(lazy:1) All changed:
95.703125

espruino/master, for reference:

(lazy:0) Initial render:
71.13647460937
(lazy:0) No changes:
71.25854492187
(lazy:0) One change:
71.01440429687
(lazy:0) All changed:
69.27490234375
(lazy:1) Initial render:
69.15283203125
(lazy:1) No changes:
71.1669921875
(lazy:1) One change:
71.044921875
(lazy:1) All changed:
71.22802734375

It looks like the bg color handling comes at some performance cost, even when it shouldn't really be doing anything. 🙁 I might see if I can optimize that more later.

On the plus side, lazy rendering doesn't appear to cost anything if you don't use it. 🙂

So I guess it reduces flicker on the Bangle 1, but I guess this wouldn't be something we did for performance unless we actually build it into the Espruino firmware as native C code.

Yeah, reducing flicker on the Bangle 1 without messy app logic is really the raison d'etre for the feature. Though I would love to see how fast it would run as native C code!

@nebbishhacker
Copy link
Author

Did a little bit of optimization.

(lazy:0) Initial render:
70.61767578125
(lazy:0) No changes:
70.61767578125
(lazy:0) One change:
72.44873046875
(lazy:0) All changed:
72.69287109375
(lazy:1) Initial render:
99.09057617187
(lazy:1) No changes:
46.35620117187
(lazy:1) One change:
61.09619140625
(lazy:1) All changed:
109.80224609375

@gfwilliams
Copy link
Member

This is great - thanks! Just merging.

I had a thought last night about how to speed up the update&render functions (switch statements aren't too speedy in Espruino), so I'll give that a try too.

Something about the minified Layout code is causing problems for me, although when that can be enabled, I'd hope there would be another bit of a performance improvement.

@gfwilliams gfwilliams merged commit edb6ba9 into espruino:master Sep 16, 2021
@gfwilliams
Copy link
Member

gfwilliams commented Sep 16, 2021

Just pushed some extra changes, so layout+render should have seen a decent speed boost now. Minification also works too which gives a minor boost.

(On Bangle.js 1)

(lazy:0) Initial render:
50.87280273437
(lazy:0) No changes:
50.23193359375
(lazy:0) One change:
52.001953125
(lazy:0) All changed:
52.24609375
(lazy:1) Initial render:
85.78491210937
(lazy:1) No changes:
47.27172851562
(lazy:1) One change:
58.9599609375
(lazy:1) All changed:
97.44262695312

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.

None yet

2 participants