Possibly wrong calculation of signed parameters

[th-otto]

I think the calculation at

https://github.com/RWAP/PrinterToPDF/blob/fd6484a2d84aded2dc2951383f10259ca77cb97d/PrinterConvert.c#L2935-L2939 is wrong.

mH and mL are both unsigned. Assuming values of mL=0x00 and mH=0x80, that would set mH to 0xff in the if, calculating a value of 65280 instead of the intended -32768.

Similar code is also used at other places like https://github.com/RWAP/PrinterToPDF/blob/fd6484a2d84aded2dc2951383f10259ca77cb97d/PrinterConvert.c#L3058

It's even worse at https://github.com/RWAP/PrinterToPDF/blob/fd6484a2d84aded2dc2951383f10259ca77cb97d/PrinterConvert.c#L1179 where a byte is read (by cast) to an integer variable. That will leave the upper 24 bits undefined on little-endian machines, and not work at all on big-endian machines.

I don't have any output file where such negative values are used, so i cannot verify it.

[RWAP]

Yes you are correct - I would suggest for the first issue,

Add line 879: signed char mX256;

Then for the two instances at lines 2935 and 3058, use:

                        mX256 = mH;
                        if (mH > 127) {
                            // Handle negative movement
                            mX256 = 127 - mH;
                        }
                        ypos2 = ypos + ((mX256 * 256) + mL) * (int) thisDefaultUnit;

This negative calculation is only used for vertical page movement so should be fine.

[th-otto]

If that is really to be treated as a signed 16bit value, that would still be wrong (calculating -256 instead of -32768).

it should imho be something like

int advance = mH * 256 + mL;
if (advance >= 32768)
   advance = advance - 65536;
ypos2 = ypos + advance * (int) thisDefaultUnit;

[RWAP]

For the second issue (line 1179), then I (think) we have to add a test for bigendian machines

int _is_little_endian_machine()
{
  unsigned int x = 1;
  char *c = (char*) &x;
  return (int)*c;
}

line 256 is currently: parameter = (unsigned char) xd & 0xF;

This could be replaced with:

    parameter = 0;
    if (_is_little_endian_machine()) {
        parameter = (unsigned char) xd & 0xF;
    } else {
        parameter = (unsigned char) xd & 0xF;
        parameter *= 1 << CHAR_BIT;
    } 

[RWAP]

If that is really to be treated as a signed 16bit value, that would still be wrong (calculating -256 instead of -32768).

it should imho be something like

int advance = mH * 256 + mL;
if (advance >= 32768)
   advance = (128 * 256) - advance;
ypos2 = ypos + advance * (int) thisDefaultUnit;

I did wonder if that made more sense - so I can see that it does

There are also a couple of instances where we use

                    if (nH > 127) {
                        // Handle negative movement
                        nH = 127 - nH;
                    }
                    xpos2 = xpos + ((nH * 256) + nL) * (int) thisDefaultUnit;

That also needs to be replaced with

                    int advance = nH * 256 + nL;
                    if (advance >= 32768) {
                        // Handle negative movement nH > 127
                        advance = (128 * 256) - advance;
                    }
                    xpos2 = xpos + advance * (int) thisDefaultUnit;

[RWAP]

I have tested the above (on a little endian machine) and also now uploaded a revised version to handle the Delta Row compression methods as there was a bug in there too.

[th-otto]

Just found an explicit statement in the manual of the LQ-2550 (as example the command to set relative horizontal position):

ESC \ Set Relative Print Position
Format:
ASCII code:  ESC \ nl n2
Decimal: 27 92 nl nh
Hexadecimal: 1B 5C n1 n2
Comments:
Determines the position (relative to the current position) at which
printing of following data will start. To find nl and nh, first calculate the
displacement required in dots. If the displacement is to the left, subtract
it from 65536. Send the resulting number using this formula: total
number of dots = nl + (256 X nh). The command is ignored if it
would move the print position outside the current margins. A unit is
1/120th of an inch in draft and 1/180th of an inch in Letter Quality or
proportional.

Subtracting the value from 65536 will indeed result in a signed 16bit value. So unfortunately, your fix in https://github.com/RWAP/PrinterToPDF/commit/3263a7a2f841d1e65b91644c5a73fdeef58e0bef still seems to be wrong, and it should be like i suggested:

advance = advance - 65536;

[RWAP]

The LQ-2550 manual appears to be different to the ESC/P2 programming standards manual, which says the figures are calculated by reference to nH = 32768 - INT (current position) / 256, nL = 32768 - MOD (current position) / 256

I would assume the programming manual is correct - but it may well be for higher density printers, they had to adapt the standard (hence why you start at 65536-value)

[th-otto]

But your current calculation does not make much sense:

                      if (advance >= 32768) {
                            // Handle negative movement mH > 127
                            advance = (128 * 256) - advance;

That will turn 32768 into 0

[RWAP]

That is correct as the documentation says that you decide the amount to move and then pass it as 32768-n

So if you passed a value of 0 (32768), that would not move the horizontal print position.

This supports nH in the range -128 to 127, nL in the range 0 to 255

(oddly the documentation also says the parameter range is 0 <= nH <= 127 (so does not take account of the negative movement!)