Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

jtoloe
Copy link

@jtoloe jtoloe commented Mar 21, 2016

… a valid colour (#80). 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 (#80)

… a valid colour (r-lib#80). NA values for colour have already been converted to completely transparent white (#ffffff00) before reaching svglite.
@codecov-io
Copy link

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
Copy link
Member

hadley commented Mar 22, 2016

Can you please provide a unit test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants