Imported PNG file not displayed.

[Tavmjong Bah]

Hi,

        In the past few days, importing a PNG file seems to have been broken.
The svg:image data seems to be correct but the file is not displayed.
Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.

                                                Tav




------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[~suv-2]

On 16/9/09 15:44, Tavmjong Bah wrote:
>
> In the past few days, importing a PNG file seems to have been broken.
> The svg:image data seems to be correct but the file is not displayed.
> Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.

Bug #429610 in Inkscape: “Imported or opened image isn't displayed”:
<https://bugs.edge.launchpad.net/inkscape/+bug/429610>

hth, ~suv

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/16/2009 04:10 PM, ~suv wrote:

On 16/9/09 15:44, Tavmjong Bah wrote:
  

	In the past few days, importing a PNG file seems to have been broken.
The svg:image data seems to be correct but the file is not displayed.
Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.
    

Bug #429610 in Inkscape: “Imported or opened image isn't displayed”:
<https://bugs.edge.launchpad.net/inkscape/+bug/429610>

Apparently this is caused by my patch which intended to eliminate some rounding errors in libnr. I hadn't noticed this new behaviour though. Please feel free to reverse this patch (rev. 22228), until I've found out what's causing this.

Diederik

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/16/2009 03:44 PM, Tavmjong Bah wrote:
> In the past few days, importing a PNG file seems to have been broken.
> The svg:image data seems to be correct but the file is not displayed.
> Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.
>  

Hi Tav, is this on a 32 bit OS?

Diederik

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Tavmjong Bah]

On Thu, 2009-09-17 at 14:17 +0200, Diederik van Lierop wrote:
> On 09/16/2009 03:44 PM, Tavmjong Bah wrote:
> > In the past few days, importing a PNG file seems to have been broken.
> > The svg:image data seems to be correct but the file is not displayed.
> > Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.
> >  
>
> Hi Tav, is this on a 32 bit OS?

Yes.

> Diederik
>



------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/17/2009 02:25 PM, Tavmjong Bah wrote:

On Thu, 2009-09-17 at 14:17 +0200, Diederik van Lierop wrote:
  

On 09/16/2009 03:44 PM, Tavmjong Bah wrote:
    

In the past few days, importing a PNG file seems to have been broken.
The svg:image data seems to be correct but the file is not displayed.
Does anybody else see this? Tested with latest SVN on Fedora 9 and 11.
  
      

Hi Tav, is this on a 32 bit OS?
    

Yes.

Thanks Tav, this once again confirms my suspicion that something is going wrong with using 64 bits long long integers on a 32 bits OS, or more likely: with the way I implemented it in libnr. This would also explain why I can't reproduce this on my 64 bit Fedora machine.

Has anyone seen this bug on a 64 bits OS?

Thanks,

Diederik



------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[bulia byak]

On 9/17/09, Diederik van Lierop <mail@...> wrote:
>  Thanks Tav, this once again confirms my suspicion that something is going
> wrong with using 64 bits long long integers on a 32 bits OS, or more likely:
> with the way I implemented it in libnr. This would also explain why I can't
> reproduce this on my 64 bit Fedora machine.

I didn't see any problems with importing on Win XP, which I believe is
32-bit, but now I notice that when you begin to zoom out on a bitmap,
below 4% zoom it starts to shift left and/or randomly disappear. Sorry
for the oversight, I have backed out that patch now.

Still, thanks to Diederik for investigating this annoying bug, now
that he has identified the source of the probelm I'm sure a correct
fix will emerge :)

--
bulia byak
Inkscape. Draw Freely.
http://www.inkscape.org

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/18/2009 08:47 PM, bulia byak wrote:

On 9/17/09, Diederik van Lierop mail@... wrote:
  

 Thanks Tav, this once again confirms my suspicion that something is going
wrong with using 64 bits long long integers on a 32 bits OS, or more likely:
with the way I implemented it in libnr. This would also explain why I can't
reproduce this on my 64 bit Fedora machine.
    

I didn't see any problems with importing on Win XP, which I believe is
32-bit, but now I notice that when you begin to zoom out on a bitmap,
below 4% zoom it starts to shift left and/or randomly disappear. Sorry
for the oversight, I have backed out that patch now.
  

This one I was able to reproduce too, on a windows XP machine. Attached patch is an improved version of my previous patch and fixes this problem, let's hope that it also fixes the issues with images not being shown.

Diederik

Index: /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp
===================================================================
--- /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp (revision 22193)
+++ /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp (working copy)
@@ -25,10 +25,10 @@
 int nr_have_mmx (void);
 void nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (unsigned char *px, int w, int h, int rs,
   const unsigned char *spx, int sw, int sh, int srs,
-  const long *FFd2s, unsigned int alpha);
+  const long long *FFd2s, unsigned int alpha);
 void nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (unsigned char *px, int w, int h, int rs,
   const unsigned char *spx, int sw, int sh, int srs,
-  const long *FFd2s, const long *FF_S, unsigned int alpha, int dbits);
+  const long long *FFd2s, const long *FF_S, unsigned int alpha, int dbits);
 #define NR_PIXOPS_MMX (1 && nr_have_mmx ())
 #ifdef __cplusplus
 }
@@ -40,6 +40,7 @@
 
 /* Fixed point precision */
 #define FBITS 12
+#define FBITS_HP 18 // In some places we need a higher precision
 
 void nr_R8G8B8A8_N_EMPTY_R8G8B8A8_N_TRANSFORM (unsigned char *px, int w, int h, int rs,
        const unsigned char *spx, int sw, int sh, int srs,
@@ -168,10 +169,10 @@
 static void
 nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (unsigned char *px, int w, int h, int rs,
  const unsigned char *spx, int sw, int sh, int srs,
- const long *FFd2s, unsigned int alpha)
+ const long long *FFd2s, unsigned int alpha)
 {
- unsigned char *d0;
- int FFsx0, FFsy0;
+    unsigned char *d0;
+ long long FFsx0, FFsy0;
  int x, y;
 
  d0 = px;
@@ -180,15 +181,15 @@
 
  for (y = 0; y < h; y++) {
  unsigned char *d;
- long FFsx, FFsy;
+ long long FFsx, FFsy;
  d = d0;
  FFsx = FFsx0;
  FFsy = FFsy0;
  for (x = 0; x < w; x++) {
  long sx, sy;
- sx = FFsx >> FBITS;
+ sx = long(FFsx >> FBITS_HP);
  if ((sx >= 0) && (sx < sw)) {
- sy = FFsy >> FBITS;
+ sy = long(FFsy >> FBITS_HP);
  if ((sy >= 0) && (sy < sh)) {
  const unsigned char *s;
  unsigned int a;
@@ -224,11 +225,11 @@
 static void
 nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (unsigned char *px, int w, int h, int rs,
  const unsigned char *spx, int sw, int sh, int srs,
- const long *FFd2s, const long *FF_S, unsigned int alpha, int dbits)
+ const long long *FFd2s, const long *FF_S, unsigned int alpha, int dbits)
 {
  int size;
  unsigned char *d0;
- int FFsx0, FFsy0;
+ long long FFsx0, FFsy0;
  int x, y;
 
  size = (1 << dbits);
@@ -242,7 +243,7 @@
 
  for (y = 0; y < h; y++) {
  unsigned char *d;
- long FFsx, FFsy;
+ long long FFsx, FFsy;
  d = d0;
  FFsx = FFsx0;
  FFsy = FFsy0;
@@ -252,9 +253,9 @@
  r = g = b = a = 0;
  for (i = 0; i < size; i++) {
  long sx, sy;
- sx = (FFsx + FF_S[2 * i]) >> FBITS;
+ sx = (FFsx >> FBITS_HP) + (FF_S[2 * i] >> FBITS);
  if ((sx >= 0) && (sx < sw)) {
- sy = (FFsy + FF_S[2 * i + 1]) >> FBITS;
+ sy = (FFsy >> FBITS_HP) + (FF_S[2 * i + 1] >> FBITS);
  if ((sy >= 0) && (sy < sh)) {
  const unsigned char *s;
  unsigned int ca;
@@ -302,6 +303,7 @@
 {
  int dbits;
  long FFd2s[6];
+ long long FFd2s_HP[6]; // with higher precision
  int i;
 
  if (alpha == 0) return;
@@ -310,17 +312,18 @@
 
  for (i = 0; i < 6; i++) {
  FFd2s[i] = (long) (d2s[i] * (1 << FBITS) + 0.5);
+ FFd2s_HP[i] = (long long) (d2s[i] * (1 << FBITS_HP) + 0.5);;
  }
 
  if (dbits == 0) {
 #ifdef WITH_MMX
  if (NR_PIXOPS_MMX) {
  /* WARNING: MMX composer REQUIRES w > 0 and h > 0 */
- nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s, alpha);
+ nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, alpha);
  return;
  }
 #endif
- nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s, alpha);
+ nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, alpha);
  } else {
  int xsize, ysize;
  long FFs_x_x_S, FFs_x_y_S, FFs_y_x_S, FFs_y_y_S;
@@ -347,11 +350,11 @@
 #ifdef WITH_MMX
  if (NR_PIXOPS_MMX) {
  /* WARNING: MMX composer REQUIRES w > 0 and h > 0 */
- nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
+ nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, FF_S, alpha, dbits);
  return;
  }
 #endif
- nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
+ nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, FF_S, alpha, dbits);
  }
 }
 

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[bulia byak]

On 9/20/09, Diederik van Lierop <mail@...> wrote:
>  This one I was able to reproduce too, on a windows XP machine. Attached
> patch is an improved version of my previous patch and fixes this problem,
> let's hope that it also fixes the issues with images not being shown.

Can anyone please confirm that this patch is free of the no-display
problem the original one had, on the same system?

--
bulia byak
Inkscape. Draw Freely.
http://www.inkscape.org

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Tavmjong Bah]

On Sun, 2009-09-20 at 10:29 -0400, bulia byak wrote:
> On 9/20/09, Diederik van Lierop <mail@...> wrote:
> >  This one I was able to reproduce too, on a windows XP machine. Attached
> > patch is an improved version of my previous patch and fixes this problem,
> > let's hope that it also fixes the issues with images not being shown.
>
> Can anyone please confirm that this patch is free of the no-display
> problem the original one had, on the same system?

Sorry, I still see the same problem after applying the patch: no PNG or
JPG displayed with 32 bit Linux.

Tav





------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[~suv-2]

On 20/9/09 16:29, bulia byak wrote:
> On 9/20/09, Diederik van Lierop <mail@...> wrote:
>>  This one I was able to reproduce too, on a windows XP machine. Attached
>> patch is an improved version of my previous patch and fixes this problem,
>> let's hope that it also fixes the issues with images not being shown.
>
> Can anyone please confirm that this patch is free of the no-display
> problem the original one had, on the same system?

No problem on 32bit OS X 10.5.8 (which apparently has some 64bit
support), but had no problem with original patch either.

~suv

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Nicolas Dufour-5]

> De : bulia byak <buliabyak@...>

> On 9/20/09, Diederik van Lierop wrote:
> >  This one I was able to reproduce too, on a windows XP machine. Attached
> > patch is an improved version of my previous patch and fixes this problem,
> > let's hope that it also fixes the issues with images not being shown.
>
> Can anyone please confirm that this patch is free of the no-display
> problem the original one had, on the same system?

Same problem with this patch on Ubuntu 9.04, 32bits. No bitmap displayed in the previews (including OCAL) and on the canvas.
--
Nicolas



     


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/20/2009 05:15 PM, Tavmjong Bah wrote:

On Sun, 2009-09-20 at 10:29 -0400, bulia byak wrote:
  

On 9/20/09, Diederik van Lierop mail@... wrote:
    

 This one I was able to reproduce too, on a windows XP machine. Attached
patch is an improved version of my previous patch and fixes this problem,
let's hope that it also fixes the issues with images not being shown.
      

Can anyone please confirm that this patch is free of the no-display
problem the original one had, on the same system?
    

Sorry, I still see the same problem after applying the patch: no PNG or
JPG displayed with 32 bit Linux.

Tav

Thanks all for your feedback; too bad the images haven't re-appeared, but I'm not giving up yet! I'm currently setting up a virtual 32 bit Fedora installation on my 64 bit machine, because I'm sure that once I can reproduce this then it should be easy to fix.

Will keep you posted!

Diederik

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/20/2009 10:05 PM, Diederik van Lierop wrote:

On 09/20/2009 05:15 PM, Tavmjong Bah wrote:

Sorry, I still see the same problem after applying the patch: no PNG or
JPG displayed with 32 bit Linux.

Tav

Thanks all for your feedback; too bad the images haven't re-appeared, but I'm not giving up yet! I'm currently setting up a virtual 32 bit Fedora installation on my 64 bit machine, because I'm sure that once I can reproduce this then it should be easy to fix.

I've managed to reproduce the problems caused by my patch; the problems arise when the processor's MMX instructions are used. A small part of libnr  has been written in assembly code to speed up the rendering and to be able to use the MMX instructions. This code needs to be changed too to handle the long long integers. My patch erroneously pushed long long integers to this assembly code anyway, which led to this bug. Unfortunately, I cannot fix this assembly code because my only assembly experience was on a Motorola 68000 processor, about 18 years ago now ;-). For now I've fixed my patch (see the attachement) by limiting it to the non-MMX c++ code. This fixes the bitmap pixel alignment issue for some users, but not for everyone. There must be someone reading this who can port this to the assembly code too. It really is only a few lines of code! Until then this bug cannot be completely solved, .... unless we start using Cairo (?)

Diederik

PS: My new patch has been tested on Fedora 10 x86_64, on a virtualised instance of Fedora 10 i686, and on a native Windows XP installation.

Index: /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp
===================================================================
--- /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp (revision 22261)
+++ /home/diedenrezi/eclipse/InkscapeSVN/src/libnr/nr-compose-transform.cpp (working copy)
@@ -40,6 +40,7 @@
 
 /* Fixed point precision */
 #define FBITS 12
+#define FBITS_HP 18 // In some places we need a higher precision
 
 void nr_R8G8B8A8_N_EMPTY_R8G8B8A8_N_TRANSFORM (unsigned char *px, int w, int h, int rs,
        const unsigned char *spx, int sw, int sh, int srs,
@@ -168,10 +169,10 @@
 static void
 nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (unsigned char *px, int w, int h, int rs,
  const unsigned char *spx, int sw, int sh, int srs,
- const long *FFd2s, unsigned int alpha)
+ const long long *FFd2s, unsigned int alpha)
 {
- unsigned char *d0;
- int FFsx0, FFsy0;
+    unsigned char *d0;
+ long long FFsx0, FFsy0;
  int x, y;
 
  d0 = px;
@@ -180,15 +181,15 @@
 
  for (y = 0; y < h; y++) {
  unsigned char *d;
- long FFsx, FFsy;
+ long long FFsx, FFsy;
  d = d0;
  FFsx = FFsx0;
  FFsy = FFsy0;
  for (x = 0; x < w; x++) {
  long sx, sy;
- sx = FFsx >> FBITS;
+ sx = long(FFsx >> FBITS_HP);
  if ((sx >= 0) && (sx < sw)) {
- sy = FFsy >> FBITS;
+ sy = long(FFsy >> FBITS_HP);
  if ((sy >= 0) && (sy < sh)) {
  const unsigned char *s;
  unsigned int a;
@@ -224,11 +225,11 @@
 static void
 nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (unsigned char *px, int w, int h, int rs,
  const unsigned char *spx, int sw, int sh, int srs,
- const long *FFd2s, const long *FF_S, unsigned int alpha, int dbits)
+ const long long *FFd2s, const long *FF_S, unsigned int alpha, int dbits)
 {
  int size;
  unsigned char *d0;
- int FFsx0, FFsy0;
+ long long FFsx0, FFsy0;
  int x, y;
 
  size = (1 << dbits);
@@ -242,7 +243,7 @@
 
  for (y = 0; y < h; y++) {
  unsigned char *d;
- long FFsx, FFsy;
+ long long FFsx, FFsy;
  d = d0;
  FFsx = FFsx0;
  FFsy = FFsy0;
@@ -252,9 +253,9 @@
  r = g = b = a = 0;
  for (i = 0; i < size; i++) {
  long sx, sy;
- sx = (FFsx + FF_S[2 * i]) >> FBITS;
+ sx = (FFsx >> FBITS_HP) + (FF_S[2 * i] >> FBITS);
  if ((sx >= 0) && (sx < sw)) {
- sy = (FFsy + FF_S[2 * i + 1]) >> FBITS;
+ sy = (FFsy >> FBITS_HP) + (FF_S[2 * i + 1] >> FBITS);
  if ((sy >= 0) && (sy < sh)) {
  const unsigned char *s;
  unsigned int ca;
@@ -302,6 +303,7 @@
 {
  int dbits;
  long FFd2s[6];
+ long long FFd2s_HP[6]; // with higher precision
  int i;
 
  if (alpha == 0) return;
@@ -310,6 +312,7 @@
 
  for (i = 0; i < 6; i++) {
  FFd2s[i] = (long) (d2s[i] * (1 << FBITS) + 0.5);
+ FFd2s_HP[i] = (long long) (d2s[i] * (1 << FBITS_HP) + 0.5);;
  }
 
  if (dbits == 0) {
@@ -320,7 +323,7 @@
  return;
  }
 #endif
- nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s, alpha);
+ nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_0 (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, alpha);
  } else {
  int xsize, ysize;
  long FFs_x_x_S, FFs_x_y_S, FFs_y_x_S, FFs_y_y_S;
@@ -351,7 +354,7 @@
  return;
  }
 #endif
- nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
+ nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s_HP, FF_S, alpha, dbits);
  }
 }
 

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[bulia byak]

The latest patch works fine for me, no shifting and no disappearance.
What would others say?

--
bulia byak
Inkscape. Draw Freely.
http://www.inkscape.org

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[~suv-2]

On 24/9/09 05:31, bulia byak wrote:
> The latest patch works fine for me, no shifting and no disappearance.
> What would others say?
>
no shifting and no disappearance on 32bit OS X 10.5.8

~suv


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Nicolas Dufour-5]

> De : bulia byak <buliabyak@...>

> The latest patch works fine for me, no shifting and no disappearance.
> What would others say?

Works fine on Ubuntu 9.04 32bits.
Thanks!
--
Nicolas



     


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Jasper van de Gronde]

Diederik van Lierop wrote:
> ... For now I've fixed my patch (see the attachement) by limiting it to
> the non-MMX c++ code. This fixes the bitmap pixel alignment issue for
> some users, but not for everyone. There must be someone reading this who
> can port this to the assembly code too. It really is only a few lines of
> code! Until then this bug cannot be completely solved, .... unless we
> start using Cairo (?)

I would recommend timing to see whether the MMX code is (still) really
much faster than the normal C++ code. If it's not significantly faster
than the old 32bit C++ code I would go with the higher quality code.

As for porting the code to assembly, if it is indeed making use of MMX I
have doubts about the feasibility/usefulness of porting the code, as the
assembly would likely not be as efficient for 64bit integers as it would
be for 32bit integers (assuming the right instructions do exist). But of
course this would have to be tested.

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/24/2009 08:52 AM, Jasper van de Gronde wrote:

> Diederik van Lierop wrote:
>> ... For now I've fixed my patch (see the attachement) by limiting it
>> to the non-MMX c++ code. This fixes the bitmap pixel alignment issue
>> for some users, but not for everyone. There must be someone reading
>> this who can port this to the assembly code too. It really is only a
>> few lines of code! Until then this bug cannot be completely solved,
>> .... unless we start using Cairo (?)
>
> I would recommend timing to see whether the MMX code is (still) really
> much faster than the normal C++ code. If it's not significantly faster
> than the old 32bit C++ code I would go with the higher quality code.
>
> As for porting the code to assembly, if it is indeed making use of MMX
> I have doubts about the feasibility/usefulness of porting the code, as
> the assembly would likely not be as efficient for 64bit integers as it
> would be for 32bit integers (assuming the right instructions do
> exist). But of course this would have to be tested.
>

Yes, valid points indeed! So I prepared a patch to do some measurements
and got some interesting results: when running Fedora 10 i386
virtualized on a Fedora 10 x86_64 host, the non-mmx code actually
appears to be faster by 10 - 15%! But before we decide to kick out some
of the mmx optimalizations (only the ones that are being tested here),
we would need more measurements because my setup might not be
representative. So to all the bug reporters who had their images
disappearing due to my previous patch: I know you're running with the
mmx optimized code ;-). Please test the attached patch against rev.
22287 and report back the measurement data. This is what you should get
on the console:

...
avg. time spent rendering; with mmx -> 4.10622 msec | without mmx ->
3.5991 msec
avg. time spent rendering; with mmx -> 4.10563 msec | without mmx ->
3.59859 msec
avg. time spent rendering; with mmx -> 4.10459 msec | without mmx ->
3.59769 msec

Only the last line is relevant.

PS: Don't use this patch for production work, because everything will be
rendered twice (with and without mmx optimalisations). This will slow
down the rendering. I also tried running the non-mmx optimized code
before the other to eliminate any caching effects, but the results look
similar

If we can get rid of this specific mmx optimized assembly code, then we
can solve the misalignment in rendering the bitmaps for everyone!

Kind regards,

Diederik


Index: src/libnr/nr-compose-transform.cpp
===================================================================
--- src/libnr/nr-compose-transform.cpp (revision 22287)
+++ src/libnr/nr-compose-transform.cpp (working copy)
@@ -345,14 +345,37 @@
  }
 
 #ifdef WITH_MMX
- if (NR_PIXOPS_MMX) {
- /* WARNING: MMX composer REQUIRES w > 0 and h > 0 */
- nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
- return;
- }
+ GTimeVal t1, t2, t3;
+       static double total_mmx = 0;
+       static double total_no_mmx = 0;
+ static double n = 0;    
+
+        if (NR_PIXOPS_MMX) {            
+            g_get_current_time(&t1);
+
+            /* WARNING: MMX composer REQUIRES w > 0 and h > 0 */
+            nr_mmx_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
+
+            g_get_current_time(&t2);
+        }
 #endif
- nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
- }
+        nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_N_TRANSFORM_n (px, w, h, rs, spx, sw, sh, srs, FFd2s, FF_S, alpha, dbits);
+#ifdef WITH_MMX
+        if (NR_PIXOPS_MMX) {
+            g_get_current_time(&t3);
+            double dt12 = ((t2.tv_sec - t1.tv_sec) * G_USEC_PER_SEC + (t2.tv_usec - t1.tv_usec)) / 1000.0;
+            double dt23 = ((t3.tv_sec - t2.tv_sec) * G_USEC_PER_SEC + (t3.tv_usec - t2.tv_usec)) / 1000.0;
+            total_mmx += dt12;
+            total_no_mmx += dt23;
+            n++;
+ std::cout << "avg. time spent rendering; with mmx -> " << total_mmx/n << " msec | without mmx -> " << total_no_mmx/n << " msec" << std::endl;
+        } else {
+            std::cout << "sorry, no NR_PIXOPS_MMX" << std::endl;
+        }
+#else
+        std::cout << "sorry, no WITH_MMX" << std::endl;
+#endif
+    }
 }
 
 void nr_R8G8B8A8_P_R8G8B8A8_P_R8G8B8A8_P_TRANSFORM (unsigned char *px, int w, int h, int rs,

------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel

[Diederik van Lierop]

On 09/26/2009 12:16 AM, Diederik van Lierop wrote:

> Yes, valid points indeed! So I prepared a patch to do some
> measurements and got some interesting results: when running Fedora 10
> i386 virtualized on a Fedora 10 x86_64 host, the non-mmx code actually
> appears to be faster by 10 - 15%! But before we decide to kick out
> some of the mmx optimalizations (only the ones that are being tested
> here), we would need more measurements because my setup might not be
> representative. So to all the bug reporters who had their images
> disappearing due to my previous patch: I know you're running with the
> mmx optimized code ;-). Please test the attached patch against rev.
> 22287 and report back the measurement data. This is what you should
> get on the console:
>
> ...
> avg. time spent rendering; with mmx -> 4.10622 msec | without mmx ->
> 3.5991 msec
> avg. time spent rendering; with mmx -> 4.10563 msec | without mmx ->
> 3.59859 msec
> avg. time spent rendering; with mmx -> 4.10459 msec | without mmx ->
> 3.59769 msec
>
> Only the last line is relevant.
>
> PS: Don't use this patch for production work, because everything will
> be rendered twice (with and without mmx optimalisations). This will
> slow down the rendering. I also tried running the non-mmx optimized
> code before the other to eliminate any caching effects, but the
> results look similar
>
> If we can get rid of this specific mmx optimized assembly code, then
> we can solve the misalignment in rendering the bitmaps for everyone!

The measurements which I reported before were on a virtual machine, but
now I can confirm the negative impact of the mmx optimized code on a
native Fedora 9 installation too!


------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel@...
https://lists.sourceforge.net/lists/listinfo/inkscape-devel