Details

    • Type: Change Request (Sub-task) Change Request (Sub-task)
    • Status: Needs documenting
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None

      Description

      This sub-task is for straight lines only. Support of circles, pie graphs, etc is out of scope.

      1. antialised graph1.png
        30 kB
      2. graphafter1.png
        33 kB
      3. graphafter2.png
        43 kB
      4. graphbefore1.png
        20 kB
      5. graphbefore2.png
        27 kB
      6. mapafter1.png
        21 kB
      7. mapbefore1.png
        21 kB

        Activity

        Hide
        Noah Leaman added a comment -

        This would indeed be a nice enhancement to the current GD based approach.

        It's worth mentioning that another approach would be to move towards already developed web client based solutions:

        http://www.amcharts.com
        http://www.highcharts.com
        http://processingjs.org

        Show
        Noah Leaman added a comment - This would indeed be a nice enhancement to the current GD based approach. It's worth mentioning that another approach would be to move towards already developed web client based solutions: http://www.amcharts.com http://www.highcharts.com http://processingjs.org
        Hide
        richlv added a comment -

        will this include links in maps ?

        Show
        richlv added a comment - will this include links in maps ?
        Hide
        Alexei Vladishev added a comment - - edited

        Implemented under development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-1659, revision 34312. Ready to test.

        The change includes support of normal and bold anti-aliased lines lines for:

        • graphs
        • map connectors
        • graph X/Y axis triangles are anti-aliased as well

        See attached screenshots.

        Show
        Alexei Vladishev added a comment - - edited Implemented under development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-1659, revision 34312. Ready to test. The change includes support of normal and bold anti-aliased lines lines for: graphs map connectors graph X/Y axis triangles are anti-aliased as well See attached screenshots.
        Hide
        Toms added a comment - - edited

        (1) Regarding

        define('LINE_TYPE_NORMAL',	'0');
        define('LINE_TYPE_BOLD',	'1');
        

        in draw.inc.php

        All constants should be defined in defines.inc.php.

        Existing constants should be considered: GRAPH_ITEM_DRAWTYPE_BOLD_LINE and GRAPH_ITEM_DRAWTYPE_LINE

        + if constant value is intended to be integer, then shouldn't be in ''

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (1) Regarding define('LINE_TYPE_NORMAL', '0'); define('LINE_TYPE_BOLD', '1'); in draw.inc.php All constants should be defined in defines.inc.php. Existing constants should be considered: GRAPH_ITEM_DRAWTYPE_BOLD_LINE and GRAPH_ITEM_DRAWTYPE_LINE + if constant value is intended to be integer, then shouldn't be in '' Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (2) Regarding function lip() in draw.inc.php

        • some more descriptive name should be used;
        • PHPDoc misleading, function does not change anything, rather it returns new color ... lower index means color more close to background;
        • missing return for PHPDoc;
        • $p = round($p, 1); should be commented, why it is necessary;
        • function parameters could be more descriptive $f could be $bgColor, $t could be $fgColor, $p could be $alpha.

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (2) Regarding function lip() in draw.inc.php some more descriptive name should be used; PHPDoc misleading, function does not change anything, rather it returns new color ... lower index means color more close to background; missing return for PHPDoc; $p = round($p, 1); should be commented, why it is necessary; function parameters could be more descriptive $f could be $bgColor, $t could be $fgColor, $p could be $alpha. Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (3) Regarding function aline() in draw.inc.php

        • typo "antialised" in PHPDoc
        • name should be in camelCase, I propose as well something more descriptive like imageAntiAliasLine

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (3) Regarding function aline() in draw.inc.php typo "antialised" in PHPDoc name should be in camelCase, I propose as well something more descriptive like imageAntiAliasLine Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (4) antialised graph1.png
        Line displayed some pixels lower to actual values.

        <richlv> and notice that axis arrows show a regression

        Alexei Vladishev It appeared that standard imageline() function incorrectly rounds X/Y coordinates by silently dropping fraction part. It was replaced by a wrapper function everywhere in the code. Also fixed dashedLine() and empty right-most vertical line in graph area. FIXED in revision 34416.

        Toms CLOSED

        Show
        Toms added a comment - - edited (4) antialised graph1.png Line displayed some pixels lower to actual values. <richlv> and notice that axis arrows show a regression Alexei Vladishev It appeared that standard imageline() function incorrectly rounds X/Y coordinates by silently dropping fraction part. It was replaced by a wrapper function everywhere in the code. Also fixed dashedLine() and empty right-most vertical line in graph area. FIXED in revision 34416. Toms CLOSED
        Hide
        Toms added a comment - - edited

        (5) Division by zero for aline() function if x1=x2 and y1=y2

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (5) Division by zero for aline() function if x1=x2 and y1=y2 Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (6) practically duplicate code in aline() lines: 67 - 96 and lines: 99 - 129

        Alexei Vladishev Decided to leave as it is. CLOSED

        Show
        Toms added a comment - - edited (6) practically duplicate code in aline() lines: 67 - 96 and lines: 99 - 129 Alexei Vladishev Decided to leave as it is. CLOSED
        Hide
        Toms added a comment - - edited

        (7) for swapping variables I propose using one-liner: list($a, $b) = array($b, $a); in draw.inc.php lines:69, 70

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (7) for swapping variables I propose using one-liner: list($a, $b) = array($b, $a); in draw.inc.php lines:69, 70 Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (8)

        $yfrac = $y - floor($y);
        $yint = round($y) - round($yfrac); 
        

        $yint is the same as floor($y);

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (8) $yfrac = $y - floor($y); $yint = round($y) - round($yfrac); $yint is the same as floor($y); Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (9)
        $tmp = $y2; $y2 = $y1; $y1 = $tmp;
        $y2 unused, no need for swapping;

        in draw.inc.php line: 70 and same in line: 101 for $x2

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (9) $tmp = $y2; $y2 = $y1; $y1 = $tmp; $y2 unused, no need for swapping; in draw.inc.php line: 70 and same in line: 101 for $x2 Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (10) no need for:
        $dx = -$dx; $dy = -$dy;
        as afterwards they are used in division.

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (10) no need for: $dx = -$dx; $dy = -$dy; as afterwards they are used in division. Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        (11) $gbcolor should be in camelCase in class.cbar line: 377

        Alexei Vladishev FIXED in revision 34402

        Toms CLOSED

        Show
        Toms added a comment - - edited (11) $gbcolor should be in camelCase in class.cbar line: 377 Alexei Vladishev FIXED in revision 34402 Toms CLOSED
        Hide
        Toms added a comment - - edited

        TESTED

        See my changes in r34435, if OK merge

        Alexei Vladishev CLOSED

        Show
        Toms added a comment - - edited TESTED See my changes in r34435, if OK merge Alexei Vladishev CLOSED
        Hide
        Alexei Vladishev added a comment -

        Fixed in version pre-2.1.0, revision 34438.

        Show
        Alexei Vladishev added a comment - Fixed in version pre-2.1.0, revision 34438.
        Hide
        richlv added a comment -

        (12) to be documented in whatsnew with nice, small before-after screenshots

        Show
        richlv added a comment - (12) to be documented in whatsnew with nice, small before-after screenshots
        Hide
        richlv added a comment -

        subissues still open : 12

        Show
        richlv added a comment - subissues still open : 12

          People

          • Assignee:
            Alexei Vladishev
            Reporter:
            Alexei Vladishev
          • Votes:
            2 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: