I've decided to strip apart, refactor, optimise and unit test my little colour library (http://code.google.com/p/colorjizz/)

I've been fixing a few bugs that have come out from the unit testing, and while doing it I've been wondering about the way I'm rounding numbers.

using the following:

 
(0.5 + number) | 0
 

Seems a hell of a lot faster than using the native rounding method in each language. In fact, in PHP it's running 4 times faster when using this other method.

I'm guessing that I'm safe to use this because all my unit tests so far are passing fine. Does anyone know of a reason I should stick to the native round() methods? Can anyone explain to me (in simple terms) why round() methods are so much slower?

http://jsfiddle.net/APcME/1/

Are you're only interested in rounding to 1 decimal place?

The benefit to the native (PHP) round function is the optional number of digits it allows you to round to and the different types of rounding, i.e. round half-up, half-down, etc.

Testing for all those options is what probably accounts for the speed difference.

I use that rounding method in my noise texture generator.

I think the reason it's faster is it can be optimised better - it's an arithmetic operation rather than a function call.

From what I remember the main downside is that it doesn't work properly (by which I mean in a mathematically correct way) on negative numbers.

Ok thanks Matt.

I've just been reading that really you shouldn't have more than 1 assertion per test case. Whoops.......

 
<?php
 
require_once 'colorjizz.php';
 
class CMYTest extends PHPUnit_Framework_TestCase
{
    const DELTA = 0.002;
 
 
    public function testToHexAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toHex()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToRGBAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toRGB()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToXYZAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toXYZ()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
 
    public function testToCMYKAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toCMYK()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToYxyAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toYxy()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToCIELabAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toCIELab()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToCIELChAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toCIELCh()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
 
    public function testToHSVAndBack()
    {
      for ($c=0; $c<=1; $c+=0.05)
      {
        for ($m=0; $m<=1; $m+=0.05)
        {
          for ($y=0; $y<=1; $y+=0.05)
          {
            $cmy = CMY::create($c, $m, $y)->toHSV()->toCMY();
            $this->assertEquals($c, $cmy->c, null, self::DELTA);
            $this->assertEquals($m, $cmy->m, null, self::DELTA);
            $this->assertEquals($y, $cmy->y, null, self::DELTA);
          }
        }
      }
    }
}
 
?>
 

If anyone says explicitly that then they're probably an idiot.

Each test case should* be a test case, but use however many assertions it takes to test that case.

*It's partly just a benefit of maintainability/comprehension (so personal preference), but also a matter of not having logic in your test cases which could itself contain bugs (that then potentially obscures actual bugs).


If the code you posted was mine, I'd probably simplify the existing functions to this:

public function testToHexAndBack()
{
	$cmy = CMY::create($c, $m, $y)->toHex()->toCMY();
	$this->assertEquals($c, $cmy->c, null, self::DELTA);
	$this->assertEquals($m, $cmy->m, null, self::DELTA);
	$this->assertEquals($y, $cmy->y, null, self::DELTA);
}

And then process them like this:

function testRoundingStuff()
{
	for ($c=0; $c<=1; $c+=0.05)
	{
		for ($m=0; $m<=1; $m+=0.05)
		{
			for ($y=0; $y<=1; $y+=0.05)
			{
				testToHexAndBack($c,$m,$y);
				testToRGBAndBack($c,$m,$y);
				// etc
			}
		}
	}
}

The problem with that (I think) is that if a single assertion fails in that test, it wont run the rest.

CMYK is a bugger to unit test.

 

$cmyk = new CMYK(0.05, 0.05, 0.05, 0);
$rgb = $cmyk->toRGB()

 

rgb gives: 242, 242, 242

 

$rgb->toCMYK() gives 0, 0, 0, 0.05

 

Pretty much every other implementation of this online seems to do the same, although with photoshop:

 

CMYK (%): 5, 5, 5, 0

 

gives:

 

RGB: 239, 236, 234

 

and CMYK (%): 0, 0, 0, 5

 

gives RGB: 241, 242, 242

 

Grr.

 

 

Get better testing software, that lets you specify to keep going.

Be careful when using photoshop that you're not complicating things with colour profiles. That's a whole other headache.

I really shouldn't have got involved in colours considering I hate them.

 

Might make it so that everything is forced to go through a greyscale function before it's outputted.

You could investigate L*a*b too; from what I remember, that's what Photoshop uses internally to convert between colour types/spaces.

Your PB and I claim my 5 brick.

What about his PB?

 

(Welcome to '99)

\o/

 

I read a summary thing that one of my lecturers wrote about my assessments in a CAD class at college. He had written "you're drawings have continued to improve". He also continually goes on about what he's going to learn us, telling us that we've already drew something so it'll be easy. Amongst other cringe inducing phrases. A fucking /lecturer/. (fail)

My library can convert to/from Lab..

for example:

 
use MischiefCollective\ColorJizz\Formats\Hex;
 
// im not sure why you'd ever want to do this
echo Hex::fromString('CC0000')->toCIELab()->toHSV()->toRGB()->toHex()->toString(); // CC0000
 
// or..
use MischiefCollective\ColorJizz\Formats\CIELab;
echo CIELab::create(53, 80, 69)->toHex()->toString(); // FE0000
 

But at the moment, converting between RGB and CMYK and back first uses CMY internally.

for example..

 
 
class CMY {
  function toRGB() {
   return $this->toCMY()->toCMYK();
 }
 
}
 
class RGB {
  function toCMYK() {
    return $this->toCMY()->toRGB();
  }
}
 

I wonder if I can find a formula that does this all in a different way.. Hm.

:(

Hmm.

 

Slowly started moving it over to namespaced PHP5.3 with some unit tests and whatnot.

 

https://github.com/mikeemoo/ColorJizz-PHP

 

Still a long way to do, and a whole load of comments to add!

 

It's not very extendable the way it is, though. Thinking of restructuring it so it's got more of a plugin-style system going on.

Your extensibility looks fine, I wouldn't bother trying to make a plug-in style system, it'll just turn into a hideous mess of over-complicated checks involving method_exists and depending on how you do it, file_exists too, all of which will only slow it down.

 

The only thing I might do is make the arguments to your class methods all objects so you can type hint them and thus enforce them to be something more robust than PHP's crappy loose typing.

 

If you want to make it really easy to extend, and as you're targeting PHP 5.3, (you'll know this already of course) just make sure you're using the 'static' keyword when calling static functions and not self so they too can be overloaded / inherited. Also put things into suitably small chunks in properly scoped methods and well, that's about it.