[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: Krita brush mask AVX Optim, Unit test scalar vs vectorized.
From:       Dmitry Kazakov <dimula73 () gmail ! com>
Date:       2018-04-28 14:31:12
Message-ID: fe23a9c3-6a0a-ad43-3093-69e0e41dbecc () gmail ! com
[Download RAW message or body]

Hi, Ivan!

1) It is not much possible to compare the brushes at the level of 
KisBrush, because there are too many things and options involved. I 
would suggest to make your
KisMaskSimilarityTester to accept two KisBrushMaskApplicatorBase* 
objects and a rect with the expected bounding rect of the brush. That 
would be much easier to control.

2) The second problem is forcing the scalar version of the code. Right 
now one cannot choose the the scalar version, there is no API for that, 
but nothing stops you from making one :) Just an idea on how it can be 
implemetned:
2.1) Define a special createOptimizedClass() function that accepts not 
only 'param' but also 'forceScalarImplemetation' flag
2.2) Then the default implementation of createOptimizedClass() can be 
refactored in terms of this new function (internally it has the same 
switch for AMD CPUs)
2.3) Add a special method to KisCircleMaskGenerator, like 'void 
resetMaskApplicator(bool forceScalar)' and call it from the unittest
2.4) In the unittest just create two KisCircleMaskGenerator objects and 
ask them to create corresponding applicators

3) For QImage comparison operations I would recommend you to use 
TestUtil::compareQImages() from qimage_test_util.h. It does exactly what 
you try to implement there :)

On 28.04.2018 07:43, Iván Yossi wrote:
> Hi!
>
> I've been working on the unit test for comparing both scalar and 
> vectorized mask. I create a mask and convert it to an image to later 
> compare pixel by pixel to ensure the mask is within the error limit.
>
> The patch along side the first Gauss AVX optimization is here
> https://phabricator.kde.org/P200
>
> I'm not sure if the way I implemented it is the recomended way as I 
> force the use of scalar by initialiazing the brush with 3 spikes. The 
> names I used for variables, I'm mostly sure I adhere to the naming 
> convention but I probably skip something. This test generates an image 
> of the masks for visual aid, I made it this way so i could asses if 
> the difference is visually detectable while in production to fine tune 
> the error.
>
> The current test will pass for the Default Brush and Fail for the 
> GaussBrush as it uses my dummy vectorized code. However it should pass 
> if only the test is integrated as both mask would be generated using 
> the scalar process.
>
> I reach to you for any advice and recommendation to make the test more 
> robust.
>
> Thanks!
>
>
> Iván Yossi
> <ghevan@gmail.com>
> IRC: ivanyossi
>

-- 
Dmitry Kazakov


[Attachment #3 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <font size="-1">Hi, Ivan!<br>
      <br>
      1) It is not much possible to compare the brushes at the level of
      KisBrush, because there are too many things and options involved.
      I would suggest to make your </font><br>
    <font size="-1"><span class="gi">KisMaskSimilarityTester to accept
        two KisBrushMaskApplicatorBase* objects and a rect with the
        expected bounding rect of the brush</span>. That would be much
      easier to control.<br>
      <br>
      2) The second problem is forcing the scalar version of the code.
      Right now one cannot choose the the scalar version, there is no
      API for that, but nothing stops you from making one :) Just an
      idea on how it can be implemetned:<br>
      2.1) Define a special createOptimizedClass() function that accepts
      not only 'param' but also 'forceScalarImplemetation' flag<br>
      2.2) Then the default implementation of createOptimizedClass() can
      be refactored in terms of this new function (internally it has the
      same switch for AMD CPUs)<br>
      2.3) Add a special method to KisCircleMaskGenerator, like 'void
      resetMaskApplicator(bool forceScalar)' and call it from the
      unittest<br>
      2.4) In the unittest just create two </font><font size="-1"><font
        size="-1">KisCircleMaskGenerator objects and ask them to create
        corresponding applicators<br>
        <br>
        3) For QImage comparison operations I would recommend you to use
        TestUtil::compareQImages() from qimage_test_util.h. It does
        exactly what you try to implement there :)<br>
        <br>
      </font></font>
    <div class="moz-cite-prefix">On 28.04.2018 07:43, Iván Yossi wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:aa19dbc9-22f4-4c15-9005-168e451d7395@Spark">
      <title></title>
      <div name="messageBodySection" style="font-size: 14px;
        font-family: -apple-system, BlinkMacSystemFont, sans-serif;">Hi!
        <div><br>
        </div>
        <div>I've been working on the unit test for comparing both
          scalar and vectorized mask. I create a mask and convert it to
          an image to later compare pixel by pixel to ensure the mask is
          within the error limit.</div>
        <div><br>
        </div>
        <div>The patch along side the first Gauss AVX optimization is
          here</div>
        <div><a href="https://phabricator.kde.org/P200"
            moz-do-not-send="true">https://phabricator.kde.org/P200</a><br>
        </div>
        <div><br>
        </div>
        <div>I'm not sure if the way I implemented it is the recomended
          way as I force the use of scalar by initialiazing the brush
          with 3 spikes. The names I used for variables, I'm mostly sure
          I adhere to the naming convention but I probably skip
          something. This test generates an image of the masks for
          visual aid, I made it this way so i could asses if the
          difference is visually detectable while in production to fine
          tune the error.</div>
        <div><br>
        </div>
        <div>The current test will pass for the Default Brush and Fail
          for the GaussBrush as it uses my dummy vectorized code.
          However it should pass if only the test is integrated as both
          mask would be generated using the scalar process.</div>
        <div><br>
        </div>
        <div>I reach to you for any advice and recommendation to make
          the test more robust.</div>
        <div><br>
        </div>
        <div>Thanks!</div>
        <div><br>
        </div>
      </div>
      <div name="messageSignatureSection" style="font-size: 14px;
        font-family: -apple-system, BlinkMacSystemFont, sans-serif;"><br>
        Iván Yossi
        <div><a class="moz-txt-link-rfc2396E" \
href="mailto:ghevan@gmail.com">&lt;ghevan@gmail.com&gt;</a></div>  <div>IRC: \
ivanyossi</div>  <div><br>
        </div>
      </div>
    </blockquote>
    <br>
    <pre class="moz-signature" cols="72">-- 
Dmitry Kazakov</pre>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic