Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    Any plans to deprecate SmartClient's self written GUI layout engine with plain CSS3 features?

    Hi,
    I know that current implementation of SmartClient drags many years of improvements to let GUI be the same in various browsers, but today's browsers and CSS3 capabilities (Grid Layout, Flexbox Layout) would allow to drop many js code for VLayout HLayout implementations. Have you discussed about this internally? I think it would speed up SmartClient, because now, for each GUI component there are many computations to calculate width heights and positions of each component on screen.
    Main issue is that every component requests its or parent's height/width or other style properties which causes browsers to instantly reflow/rendering DOM while GUI is still in drawing. By that browsers cannot optimise their html/css rendering properly and that causes a slow down of GUI. It can be visually seen when application has many GUI elements, e.g. couple of Calendars with many items.
    Maybe at first VLayout HLayout can be rewritten to use Flexbox Layout model instead of custom calculations written in js? Or it's technically would be entire rewrite?

    #2
    We've been tracking this possibility for a while, but unfortunately, flexbox can do a few of the things that HLayout / VLayout can do, but not nearly the whole feature set (for example, adaptive width, scrolling anticipation, certain animations..).

    In addition flexbox is still riddled with subtle bugs. The testing of this feature has centered around reproducing certain simple columnar layouts in static content, with page-level resizing. When you get into lots of dynamic changes and deep nesting, lots of bugs become apparent.

    Furthermore, the internal mechanism of flexbox has the same characteristic of having to determine DOM sizes in order to proceed with allocating space to other elements in the same layout. It's just inherent to any such algorithm.

    Finally, if you are experiencing slow reflows in your app, you likely have issues not related to whether size calculations are taking place in JS vs internal to the browser. Every time we investigate a customer's "slow relayout" scenario we find the same set of problems: items that are really fixed size being left as flexible size (overflow:visible) hence reflowing unnecessarily, excessive numbers of components used where fewer would reproduce the same appearance and behavior, manual redraw() calls for no apparent reason (or the reason is forgotten), etc. Focus on those to improve performance; we offer Consulting services if you need additional help.

    Comment


      #3
      Thank you for an answer,
      I similarly thought about this that it might be still buggy and not useful for deep layouts.
      Maybe I've mixed layouting and slowness in my question, but actually I see slowness not in "slow relayout", but slowness on Canvas elements drawings (might be(or not) related to layout calculations), as every time they are requesting some properties of DOM (width,height,styles) instantly after draw on DOM (probably using innerHTML) to store values into Canvas objects for further calculations.

      By that, browsers cannot defer DOM rendering until the last moment and it internally draws DOM on each canvas insert. As I mentioned I've experienced on calendar component with considerable amount of EventCanvas objects. Reading more about browssers internals find out that browsers are intelligently enough to skip DOM rendering until really necessary, e.g. you can insert DOM elements, change their innerHTML's and browser will defer it's drawing until necessary (to displ
      ay on screen after some ms, if any JS requests DOM element properties etc.).

      At first I though that it's impossible to speed up until I've read about browser internals, and I came up with these performance improvements. As you can see it will disable properties read after component's draw so browsers can defer its internal renderings until the last moment.

      If you profile performance (I used Chrome) and diff with plain and this patch applied you'll notice 2-3x improvement of Calendar draawing (no reflow on each addEevent() call).
      I understand that methods overridden in this hack-y way could/will trigger some more issues, but performance increases dramatically.

      Maybe this practice could be applied to other components too? e.g. DynamicForms with many FormItems also could be improved similarly.

      Long writing, but might be clear why I'm posted this in wish list (thought that avoiding internal layout calculations would avoid eagerly reading DOM properties which limits internal browser's optimizations and by that we could increase drawing performance)

      Please take look at this performance patch and maybe some ideas can be integrated into SmartClient's core (not only calendar/eventcanvas).
      Code:
      // performance fixes begin //
      (<any>isc).EventCanvas.addProperties({
          getCurrentCursor: function () { return "pointer";},
          _browserDoneDrawing: function () { return true; },
          _testForHiddenCanvasIssue: () => false
      });
      
      isc.Timeline.addProperties({
          indicatorCanvasProperties: { notifyAncestorsOnReflow: false },
          eventCanvasLabelProperties:  { notifyAncestorsOnReflow: false },
          eventCanvasButtonLayout: { notifyAncestorsOnReflow: false }
      });
      
      (<any>isc).TimelineView.addProperties({
          _childAboutToReflow: () => false,
          dragSelectCanvasProperties: { notifyAncestorsOnReflow: false },
          eventDragTargetProperties: { notifyAncestorsOnReflow: false },
          setLanes : function (lanes, skipDataUpdate) {
              var cal = this.calendar,
                  laneNameField = cal.laneNameField;
              this.lanes = lanes.duplicate();
              var laneCount = lanes.length;
              for (var i=0; i<laneCount; i++) {
                  var lane = lanes[i];
                  if (!lane[laneNameField]) lane[laneNameField] = lane.name;
                  if (lane.sublanes) {
                      var laneHeight = this.getLaneHeight(lane),
                          len = lane.sublanes.length,
                          sublaneHeight = Math.floor(laneHeight / len),
                          offset = 0
                      ;
                      for (var j=0; j<len; j++) {
                          var sublane = lane.sublanes[j];
                          sublane[laneNameField] = sublane.name;
                          sublane.top = offset;
                          if (sublane.height == null) sublane.height = sublaneHeight;
                          offset += sublane.height;
                      }
                      lane.height = lane.sublanes.getProperty("height").sum();
                  } else {
                      lane.height = this.getLaneHeight(lane);
                  }
              }
      
              this.setData(lanes);
              // if (this.isDrawn()) this.redraw(); // comment out, skip double redraw
      
              // refetch or just redraw applicable events (setLanes() may have been called after setData)
              if (!skipDataUpdate) {
                  // if we're going to refresh data, remove the flag preventing that from happening
                  delete cal._ignoreDataChanged;
                  this._refreshData();
              }
          }
      })
      // performance fixes end //

      Comment


        #4
        As we covered previously, we ask for DOM sizes when we need them to do further layout calculations. Generally, we defer as much as possible. The browser's internal flexbox rendering algorithm has to do the same: it has to determine sizes for DOM structures in order to proceed.

        This "patch" simply breaks hundreds of automated tests, breaks various features, and disables workarounds needed on certain browsers. If this miraculously works for one narrow use case in your application, feel free to use it, however, do not report any "bugs" to us without first removing this code and re-testing.

        Comment


          #5
          Yes, but this patch will prevent browser's DOM reflow on each new event added to timeline, when calendar contains more than 500 events this patch noticeably improves performance, n-times not percents.
          For example: to call getCurrentCursor() method on each event insert? why? what is the point? (this will trigger DOM reflow immediately)
          or _browserDoneDrawing()? Is this necessary on latest browsers? And call on each calendar item insert (bulk insert)

          I understand that this is important for proper SmartClient's workings and it's ok when Canvas items is not too much. Component like Calendar creates a lot of Canvas objects to represent its items on screen and these methods (which I've mocked) causes browsers to re-render DOM on each item add. Without these methods browser is smart enough and renders DOM only after all inserts.

          Yes, architecturally it would be hard to implement some kind of transactional GUI deferral mechanism, but maybe these components (as Calendar) could be implemented without using isc.Canvas, something "lightweight Canvas" objects could be used. Just thoughts.

          Comment


            #6
            So again, your "patch" simply breaks the framework. If it wasn't clear before, we have thousands of automated tests, and applying just pieces of your "patch" causes massive, widespread breakage.

            And again, we already, in general, have systems in place to minimize reflow. We intentionally do not call _browserDoneDrawing until we need sizing information, allowing large numbers of children to render together without reflows.

            You've never shared code for the application that this "patch" speeds up, and it's very possible that it shows an approach that induces additional reflows, making the "patch" plus the application a "two wrongs make a right" situation, which in turn makes it pointless to even look into your "patch".

            If you're interested in contributing something that could lead to an optimization, we would need a runnable test case of Calendar usage that's slow and is sped up by temporary hacks that skip reflows. Then, we could tell you how to either adjust your application code, or potentially apply a correct and usable approach to skipping reflows that might help specific scenarios in the Calendar.

            Comment


              #7
              I've tried to implement CSS Grid Layout and started doing it as a completely separate component (subclassing Canvas like the Layout class does). It isn't ready yet but my experience doing so confirms that neither Flexbox or CSS Grid would be 'drop-in' replacements for VLayout or HLayout in all cases. However, I figured a separate component could be a useful base to build new components on top of without risking breakage of existing functionality.

              I started making a new DynamicForm layout based on my CSSGridLayout class and the results were promising (but required lots of work). Had to drop the work while I focussed on other issues but will hopefully come back to it.

              Comment

              Working...
              X