eclipsesource / tabris-js

Create native mobile apps in JavaScript or TypeScript.
https://tabrisjs.com
BSD 3-Clause "New" or "Revised" License
1.4k stars 170 forks source link

Slower performances of collectionView with a lot of columnCount #2057

Open ishigo1987 opened 4 years ago

ishigo1987 commented 4 years ago

When the collectionView has a high number of columns a significant slowdown in performance is felt during scrolling. The problem does not come from the number of items inserted at once but from the number of columns displayed.

Applications such as WHatsapp, Telegram etc... do not have this problem.

Expected behavior

fast scrolling of the elements of the collectionView

Environment

Code snippet

const{CollectionView, TextView, contentView} = require("tabris");
// emojisList is an array with 2000 emojis characters
new CollectionView({ top: 45, left: 0, right: 0, bottom: 0, itemCount:emojisList.length, columnCount: 8, cellHeight: 40, 
           createCell: () => {
              const cell = new Composite({ width:40, height:40, class:'cell'});
              new TextView({centerX:0, centerY:0, id:"emoji"}).appendTo(cell);
              cell.onTap(()=>{
              });
                 return cell;
              },updateCell: (cell,index) =>{
                  cell.find('#emoji').only().text = emojisList[index].emoji;

              }
        }).appendTo(.contentView);
ishigo1987 commented 4 years ago

Hnet-image

8 columns

ishigo1987 commented 4 years ago

Hnet-image (1)

One column

fax1ty commented 4 years ago

Not sure, but I think you just have too many items. Did you check example in Slack?

ishigo1987 commented 4 years ago

I don't think the problem is the number of items I take an example I can display 5000 photos from my gallery in 2 columns without performance degradation, the emojis on the gif are only text and I only display 1000 so 5 times less, the difference is the number of columns

ishigo1987 commented 4 years ago

if i'm not wrong slack use a png image like emoji, and despite the fact that it's pictures and not text as in my case it's faster and more fluid.

ishigo1987 commented 4 years ago

slack has a different approach than whatsapp or Telegram they don't use a collectionView but instead a scrollView with horizontal scroll

mpost commented 4 years ago

Could it be that drawing the emoji glyph itself is a costly operation?

ishigo1987 commented 4 years ago

Maybe indeed but I have a little doubt, I made the same experience with 2000 png images of emojis and 8 columns, the performances were worse, the application crashed at one point.

mpost commented 4 years ago

Can you provide a self contained snippet including the data your are trying to display?

ishigo1987 commented 4 years ago

yes i do

{
        "no":358,
        "code":"U+1F4AA",
        "emoji":"💪",
        "description":"FLEXED BICEPS",
        "flagged":false,
        "keywords":[
          "biceps",
          "body",
          "comic",
          "flex",
          "muscle"
        ],
        "types":[
          "U+1F4AA U+1F3FF",
          "U+1F4AA U+1F3FE",
          "U+1F4AA U+1F3FD",
          "U+1F4AA U+1F3FC",
          "U+1F4AA U+1F3FB"
        ]
      }

i've 1788 items emojis characters with the key emoji

ishigo1987 commented 4 years ago
 new CollectionView({ top: 45, left: 0, right: 0, bottom: 0, itemCount:emojisList.length, columnCount: 8, cellHeight: cellHeight, 
            createCell: () => {
               const textEmojis = new TextView({centerY:0, alignment:"centerX", font:"26px emojis", id:"emoji"})
               .onTap(({target})=>{
                 const index = target.parent(CollectionView).itemIndex(target);
               });
                  return textEmojis;
               },updateCell: (textEmojis,index) =>{
                   textEmojis.text = emojisList[index].emoji;

               }
         }).appendTo(areaEmoji);
cookieguru commented 4 years ago

@ishigo1987 That isn't a self-contained snippet as areaEmoji is undefined

ishigo1987 commented 4 years ago

@cookieguru my apologizes

const areaEmoji = new Composite({ bottom:0, left:0, right:0, height:250, background:"#f5f5f5"}).appendTo(contentView);
ishigo1987 commented 4 years ago

Any news?

fax1ty commented 4 years ago

@ishigo1987 Your code is very unclear. I think there are two reasons why everything is lagging:

  1. You are doing something wrong
  2. Your device is too old

Everything is pretty smooth with this code:

import { contentView, CollectionView, TextView } from 'tabris';

let emojis = new Array(150).fill('\u{1F638}');

new CollectionView({
  left: 0, right: 0, top: 0, bottom: 0,
  itemCount: emojis.length,
  createCell: () => new TextView({ left: 0, right: 0, padding: 25, alignment: 'centerX' }),
  updateCell: (cell, i) => cell.text = emojis[i]
})
  .appendTo(contentView);

ezgif com-video-to-gif

fax1ty commented 4 years ago

I wrote and tested a fully working example of the design you need in Slack. Why not just use it?

ishigo1987 commented 4 years ago

@fax1ty You don't understand my problem, what you're doing now I explained it above, the problem is not my code, the code you just published I showed it on one of my gifs you used only one column for the collectionView, with only one column it's fluid I said it, now replace in your code columnCount = 1 by columnCount = 8 and see if it's still fluid

ishigo1987 commented 4 years ago

@fax1ty the problem it's not the design,the problem is the collectionView number of columns, the higher it is the more the collectionView loses in performances

cookieguru commented 4 years ago

I think column count is not the issue per se, it's the number of cells redrawing at a given time. With 8 columns you can have 16 widgets being redrawn at the same time. Check the performance of the same code that simultaneously updates 16 TextViews not part of a CollectionView and I bet you'd see similar lag

ishigo1987 commented 4 years ago

In this case how do applications like whatsapp or telegram manage to redesign 16 widgets without loss of performance?that's the question we should ask ourselves I think

cookieguru commented 4 years ago

@fax1ty already explained it: they use a ScrollView where all the widget's children are rendered as soon as the component is initialized, avoiding the problem of repeatedly changing the cell's contents

ishigo1987 commented 4 years ago

ok this is something I'm going to explore, but I'm not sure if I can just in a scrollView to be able to align the elements on 8 columns like in a collectionView, I have to admit that at the moment I don't see how to do it.

cookieguru commented 4 years ago

Try RowLayout

ishigo1987 commented 4 years ago

@fax1ty @cookieguru @mpost excuse me if I'm disturbing you, but can you please tell me how I should make the layout succeed to display in a scrollView successive TextViews and on 9 columns as it would be the case for a collectionView? Javascript please.

ishigo1987 commented 4 years ago

i already try with Rowlayout with no success, maybe i'm missing something

cookieguru commented 4 years ago

I don't have the psychic module installed 😛 maybe you can share some code?

ishigo1987 commented 4 years ago

Psychic Module :v :v :v , I tried this the day before yesterday and erased it. I'll redo it and send it today. Now I'm doing some experiments and I realize that Tabris is able to display 100 textviews via a for loop in a scrollView in less than a second, after 100 the delay before the display becomes a little long .

ishigo1987 commented 4 years ago
const{TextView, contentView, ScrollView} = require("tabris");
const scrollView = new ScrollView({layoutData:"stretch"}).appendTo(contentView);
for(let i=0; i<100; i++){
    new TextView({font:"16px", alignment:"left", top:["prev()", 10], text:`this is the number ${i}`}).appendTo(scrollView)
}
cookieguru commented 4 years ago

In that snippet every TextView has identical layout data so you'll have 100 TextViews in the same position inside the ScrollView

ishigo1987 commented 4 years ago

yes you're right here the goal for me was to see how many textviews I can display per batch in the scrollview without any noticeable delay for the user

ishigo1987 commented 4 years ago

A draft

const{TextView, contentView, ScrollView, Composite, RowLayout} = require("tabris");
const scrollView = new ScrollView({layoutData:"stretch",layout: new RowLayout({spacing: 16, alignment: 'left'})}).appendTo(contentView);
const cellHeight = Number(screen.width / 8)
for(let i=0; i<100; i++){
    new Composite({width:cellHeight, height:cellHeight, left:["prev()",0], highlightOnTouch:true}).append(
       new TextView({font:"28px", centerX:0, centerY:0, top:["prev()", 10], text:i}).appendTo(scrollView)
    ).appendTo(scrollView)
}
ishigo1987 commented 4 years ago

Screenshot_20200515-084847 The result, my problem is how to go to the next line

cookieguru commented 4 years ago
const {TextView, contentView, ScrollView, Composite, RowLayout} = require("tabris");
const scrollView = new ScrollView({layoutData: "stretch"}).appendTo(contentView);
const ROWS = 18;
const COLUMNS = 8;
const CELL_SIZE = Number(screen.width / COLUMNS);
for(let y = 0; y < ROWS; y++) {
    for(let x = 0; x <= COLUMNS; x++) {
        new TextView({font: "28px", top: y * CELL_SIZE, left: x * CELL_SIZE, text: '🙂'}).appendTo(scrollView)
    }
}

Screenshot

fax1ty commented 4 years ago

@ishigo1987 I didn't even know there was a property like columnCount. I provided my own code for splitting into rows and columns. It runs smoothly and does what it needs to. I still don't understand what you want to achieve. Why are you still looking for another solution?

If you so don't want to use my code, as soon as I can I will test the columnCount option

ishigo1987 commented 4 years ago

@fax1ty I'm afraid I haven't seen your code, if you're talking about the one on slack it was made with a view collection I believe.Did you make a code just with a scrollView like the one from @cookieguru

ishigo1987 commented 4 years ago

@cookieguru Thank You dude, you save my day

fax1ty commented 4 years ago

@ishigo1987

Why is the ScrollView solution worse than CollectionView:

  1. Now you need to calculate where to scroll (I'm talking about categories)
  2. Now all copies of TextView exist simultaneously

I checked the following code:

import { contentView, CollectionView, TextView } from 'tabris';

let emojis = new Array(150).fill('\u{1F638}');

new CollectionView({
  left: 0, right: 0, top: 0, bottom: 0,
  itemCount: emojis.length,
  createCell: () => new TextView({ left: 0, right: 0, padding: 25, alignment: 'centerX' }),
  updateCell: (cell, i) => cell.text = emojis[i],
  columnCount: 6
})
  .appendTo(contentView);

Everything works quite smoothly

ezgif com-video-to-gif (1)

Since

image

After I did .appendTo() 3404 times at the same time my app completely froze and crashed with ANR

CollectionView is fine even with 3000+ items

ishigo1987 commented 4 years ago

@fax1ty Your sample code works very well but it is not very close to my needs. try this one and you'll see that the performance goes down.

import { contentView, CollectionView, TextView } from 'tabris';
let emojis = new Array(500).fill('\u{1F638}');

new CollectionView({
  left: 0, right: 0, top: 0, bottom: 0,
  itemCount: emojis.length,
  createCell: () => new TextView({ font:'28px', left: 0, right: 0, alignment: 'centerX' }),
  updateCell: (cell, i) => cell.text = emojis[i],
  columnCount: 6
}).appendTo(contentView);
fax1ty commented 4 years ago

@mpost The real problem may be caused by specifying the size of the text. As you can see, my code and @ishigo1987's code differ in one property - font. Performance drops noticeably after I set the font size

@ishigo1987 For your needs, I wrote a special code that works even with 500 elements. I think it will work even with 3000 elements. And you can't explain why it doesn't fit you/why you don't want to use it

import { contentView, CollectionView, TextView, Composite, device, ScrollView, ImageView } from 'tabris';
let firstEmojiGroup = new Array<string>(200).fill('\u{1F408}');
let secondEmojGroup = new Array<string>(300).fill('\u{1F43B}');
let chunkCount = 6;
let emojiViewPadding = 15;
let categorizedData = [{ items: firstEmojiGroup, icon: '' }, { items: secondEmojGroup, icon: '' }];
let data = firstEmojiGroup.concat(secondEmojGroup);
let chunkedData = chunk(data, chunkCount);
let emojiSize = Math.ceil((device.screenWidth - emojiViewPadding * 2 * (chunkCount + 1)) / chunkCount);
let menu = new ScrollView({ left: 0, right: 0, direction: 'horizontal', scrollbarVisible: false, padding: 15 })
  .append(
    categorizedData.map((c, i) =>
      new ImageView({ width: 25, height: 25, cornerRadius: 25 / 4, background: i == 0 ? 'blue' : '#eee', tintColor: i == 0 ? 'blue' : '#eee', image: c.icon, highlightOnTouch: true, left: i == 0 ? 0 : 'prev() 25' })
        .onTap(({ target: icon }) => {
          icon.parent().children<ImageView>().forEach(i => { i.tintColor = '#eee'; i.background = '#eee'; });
          icon.tintColor = 'blue';
          icon.background = 'blue';
          cv.reveal(chunkedData.findIndex(chunk => chunk.find(item => item == categorizedData[i].items[0])), { animate: true });
        })
    )
  )
  .appendTo(contentView);
let cv = new CollectionView({
  left: 0, right: 0, top: 'prev() 0', bottom: '50%',
  itemCount: chunkedData.length,
  createCell: () => new Composite({ left: 0, right: 0 })
    .append(ev(chunkCount)),
  updateCell: (cell, i) => {
    let emojiViews = cell.children<TextView>();
    chunkedData[i].forEach((e, ei) => emojiViews[ei].text = e);
  }
})
  .onLastVisibleIndexChanged(({ value }) => {
    let n = categorizedData.findIndex(category => category.items.find(item => item == chunkedData[value][0]));
    let icon = menu.children<ImageView>()[n];
    icon.parent().children<ImageView>().forEach(i => { i.tintColor = '#eee'; i.background = '#eee'; });
    icon.tintColor = 'blue';
    icon.background = 'blue';
  })
  .appendTo(contentView);
function chunk<T>(arr: Array<T>, size: number) {
  let temp = [];
  for (let i = 0; i < arr.length; i += size) {
    temp.push(arr.slice(i, i + size));
  }
  return temp;
}
function ev(count: number) {
  let temp = [];
  for (let i = 0; i < count; i++) {
    temp.push(new TextView({ font: `${emojiSize}px`, left: 'prev() 0', highlightOnTouch: true, padding: emojiViewPadding, cornerRadius: emojiSize / 2 }));
  }
  return temp;
}
new TextView({ top: 'prev() 25', left: 25, right: 25, padding: 25, background: '#eee', text: 'Go to the end', alignment: 'centerX', highlightOnTouch: true, cornerRadius: 16 })
  .onTap(() => cv.reveal(-1, { animate: true }))
  .appendTo(contentView);
ishigo1987 commented 4 years ago

Ok thank you @fax1ty I'm looking at it now.

ishigo1987 commented 4 years ago

@fax1ty I tried your solution, it works pretty well but if I increase the number of columns to 8 the performance drops, I think the tabris team should see this problem more closely, what do you think @cookieguru @mpost is it a tabris optimization problem or a problem with the android platform?, i talk about this from @cookieguru I think column count is not the issue per se, it's the number of cells redrawing at a given time. With 8 columns you can have 16 widgets being redrawn at the same time. Check the performance of the same code that simultaneously updates 16 TextViews not part of a CollectionView and I bet you'd see similar lag

cookieguru commented 4 years ago

Have you tried that?

ishigo1987 commented 4 years ago

Yes, now i test your solution too

mpost commented 4 years ago

I have tested the snippet from @fax1ty given here as well has the small one from @ishigo1987. Both performed very well when scrolling on tabris 3.5 with the emulator as well as a Pixel 4.

Screenshot_1589790715

The screenshot shows the time it takes to draw one frame while scrolling. All frames are well under the red line (16ms) so it is a smooth scroll experience.

ishigo1987 commented 4 years ago

ok Thank You so if i move to Tabris 3.3 to Tabris 3.5 i will have a performance boost?Tabris 3.5 is based on J2v8 version 6.1?

mpost commented 4 years ago

3.4 does use j2v8 6.1. This will boost js performance. The scroll performance of the CV could be effected but i wouldn't think that it will be a drastic change.

ishigo1987 commented 4 years ago

ok thank you, one last question if you allow me, J2v8 6.1 is based on which version on V8?

ahmadov commented 4 years ago

@ishigo1987 J2V8 6.1 is based on the V8 version 7.4.288

ishigo1987 commented 4 years ago

Thank you @ahmadov