r-lib / svglite

A lightweight svg graphics device for R
https://svglite.r-lib.org
180 stars 39 forks source link

The checks for NA_INTEGER are unnecessary. The value of NA_INTEGER is… #63

Closed jtoloe closed 8 years ago

jtoloe commented 8 years ago

… a valid colour (#00000080). NA values for colour have already been converted to completely transparent white (#ffffff00) before reaching svglite.

See R_ext/GraphicsDevice.h where it says:

/*
 * Changes as from 2.0.0:  use top 8 bits as full alpha channel
 *      255 = opaque, 0 = transparent
 *      [to conform with SVG, PDF and others]
 *      and everything in between is used
 *      [which means that NA is not stored as an internal colour;
 *       it is converted to R_RGBA(255, 255, 255, 0)]
 */

So a check for NA_INTEGER on a colour is meaningless since it will match a different colour (#00000080)

codecov-io commented 8 years ago

Current coverage is 93.77%

Merging #63 into master will not affect coverage as of 4a076a0

@@            master     #63   diff @@
======================================
  Files            6       6       
  Stmts          466     466       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            437     437       
  Partial          0       0       
  Missed          29      29       

Review entire Coverage Diff as of 4a076a0

Powered by Codecov. Updated on successful CI builds.

hadley commented 8 years ago

Can you please provide a unit test?