From 36e3aac85ef60e39371d848bebcac265981d87f2 Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Wed, 30 Apr 2014 09:47:33 +0200 Subject: [PATCH 1/2] [IMP] large optimizations to the graph view rendering (pivot table) The pivot table rendering was slow with large tables : it appended to the table each line, so the browser had to rerender each time the page. This was solved by rendering the table to a document fragment, then only add the fragment to the main document. This speeds up considerably the process (from 2600ms to 420ms in one test) Also, uses basic loops instead of underscore maps in the build_rows method bzr revid: ged@openerp.com-20140430074733-quhfkzz33rlpahps --- .../web_graph/static/src/js/graph_widget.js | 111 +++++++++++------- 1 file changed, 69 insertions(+), 42 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index 317ea6cbfb9..d6f9b929dea 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -453,45 +453,55 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ build_rows: function (raw) { var self = this, pivot = this.pivot, - m, i, cell; + m, i, j, k, cell, row; - return _.map(pivot.rows.headers, function (row) { - var cells = []; - var pivot_cells = []; - for (i = 0; i < pivot.cells.length; i++) { - if (pivot.cells[i].x == row.id || pivot.cells[i].y == row.id) { - pivot_cells.push(pivot.cells[i]); - } + var rows = []; + var cells, pivot_cells, values; + + var nbr_of_rows = pivot.rows.headers.length; + var col_headers = pivot.get_cols_leaves(); + + for (i = 0; i < nbr_of_rows; i++) { + row = pivot.rows.headers[i]; + cells = []; + pivot_cells = []; + for (j = 0; j < pivot.cells.length; j++) { + if (pivot.cells[j].x == row.id || pivot.cells[j].y == row.id) { + pivot_cells.push(pivot.cells[j]); + } } - _.each(pivot.get_cols_leaves(), function (col) { - var values; - for (i = 0; i < pivot_cells.length; i++) { - if (pivot_cells[i].x == col.id || pivot_cells[i].y == col.id) { - values = pivot_cells[i].values; + + for (j = 0; j < col_headers.length; j++) { + values = undefined; + for (k = 0; k < pivot_cells.length; k++) { + if (pivot_cells[k].x == col_headers[j].id || pivot_cells[k].y == col_headers[j].id) { + values = pivot_cells[k].values; break; - } + } } if (!values) { values = new Array(pivot.measures.length);} for (m = 0; m < pivot.measures.length; m++) { - cells.push(self.make_cell(row,col,values[m], m, raw)); + cells.push(self.make_cell(row,col_headers[j],values[m], m, raw)); } - }); - if (pivot.get_cols_leaves().length > 1) { + } + if (col_headers.length > 1) { var totals = pivot.get_total(row); for (m = 0; m < pivot.measures.length; m++) { - cell = self.make_cell(row, pivot.main_col(), totals[m], m, raw); + cell = self.make_cell(row, pivot.cols.headers[0], totals[m], m, raw); cell.is_bold = 'true'; cells.push(cell); } } - return { + rows.push({ id: row.id, indent: row.path.length, title: row.title, expanded: row.expanded, cells: cells, - }; - }); + }); + } + + return rows; }, // ---------------------------------------------------------------------- @@ -524,9 +534,11 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ // ---------------------------------------------------------------------- draw_table: function () { var table = this.build_table(); - this.draw_headers(table.headers); - this.draw_measure_row(table.measure_row); - this.draw_rows(table.rows); + var doc_fragment = $(document.createDocumentFragment()); + this.draw_headers(table.headers, doc_fragment); + this.draw_measure_row(table.measure_row, doc_fragment); + this.draw_rows(table.rows, doc_fragment); + this.table.append(doc_fragment); }, make_header_cell: function (header) { @@ -549,7 +561,7 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ return cell.append(content); }, - draw_headers: function (headers) { + draw_headers: function (headers, doc_fragment) { var make_cell = this.make_header_cell, empty_cell = $('').attr('rowspan', headers.length), thead = $(''); @@ -562,10 +574,11 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ thead.append(html_row); }); thead.children(':first').prepend(empty_cell); - this.table.append(thead); + doc_fragment.append(thead); + this.thead = thead; }, - draw_measure_row: function (measure_row) { + draw_measure_row: function (measure_row, doc_fragment) { if (this.pivot.measures.length === 1) { return; } var html_row = $('').append(''); _.each(measure_row, function (cell) { @@ -573,25 +586,39 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ if (cell.is_bold) {measure_cell.css('font-weight', 'bold');} html_row.append(measure_cell); }); - this.$('thead').append(html_row); + this.thead.append(html_row); }, - draw_rows: function (rows) { - var table = this.table, - make_cell = this.make_header_cell; + draw_rows: function (rows, doc_fragment) { + var make_cell = this.make_header_cell, + cell, + html_row, i, j; - _.each(rows, function (row) { - var html_row = $('').append(make_cell(row)); - _.each(row.cells, function (cell) { - var html_cell = $('').text(cell.value); - if (_.has(cell, 'color')) { - html_cell.css('background-color', $.Color(255, cell.color, cell.color)); + var cells_list, + hcell, + rows_length, cells_length; + + rows_length = rows.length; + for (i = 0; i < rows_length; i++) { + html_row = $('').append(make_cell(rows[i])); + cells_length = rows[i].cells.length; + cells_list = []; + + for (j = 0; j < cells_length; j++) { + cell = rows[i].cells[j]; + hcell = ''; + cells_list[j] = hcell; + } + html_row.append(cells_list.join('')); + doc_fragment.append($('').append(html_row)); + } }, // ---------------------------------------------------------------------- From 1f693aaed6f6e28a384f20d357f8a3f558d33f6a Mon Sep 17 00:00:00 2001 From: Gery Debongnie Date: Wed, 30 Apr 2014 10:49:27 +0200 Subject: [PATCH 2/2] [FIX] fix a crash in graphview when user clicked rapidly twice on menu item The issue was that the get_search_fields method tried to get the view's corresponding searchview after a rpc (fields_get). Sadly, if the user had meanwhile clicked again on the menu item, the first view would be detached from the view manager and would be unable to reach the search view. The solution is to move the lookup for the search view in the start method, where it is guaranteed to exist. bzr revid: ged@openerp.com-20140430084927-m11dxqg9ko0dnu08 --- addons/web_graph/static/src/js/graph_widget.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/addons/web_graph/static/src/js/graph_widget.js b/addons/web_graph/static/src/js/graph_widget.js index d6f9b929dea..84ca75ddda2 100644 --- a/addons/web_graph/static/src/js/graph_widget.js +++ b/addons/web_graph/static/src/js/graph_widget.js @@ -43,6 +43,13 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ this.$('.graph_main_content').addClass('graph_pivot_mode'); } + // get search view + var parent = this.getParent(); + while (!(parent instanceof openerp.web.ViewManager)) { + parent = parent.getParent(); + } + this.search_view = parent.searchview; + openerp.session.rpc('/web_graph/check_xlwt').then(function (result) { self.$('.graph_options_selection label').toggle(result); }); @@ -76,16 +83,9 @@ openerp.web_graph.Graph = openerp.web.Widget.extend({ // this method gets the fields that appear in the search view, under the // 'Groupby' heading get_search_fields: function () { - var self = this, - parent = this.getParent(); + var self = this; - while (!(parent instanceof openerp.web.ViewManager)) { - parent = parent.getParent(); - } - - var search_view = parent.searchview; - - var groupbygroups = _(search_view.inputs).select(function (g) { + var groupbygroups = _(this.search_view.inputs).select(function (g) { return g instanceof openerp.web.search.GroupbyGroup; });