lindell / JsBarcode

Barcode generation library written in JavaScript that works in both the browser and on Node.js
http://lindell.me/JsBarcode
MIT License
5.5k stars 1.11k forks source link

Support RM4SCC and KIX #130

Open abmaonline opened 7 years ago

abmaonline commented 7 years ago

I've been working on KIX support (Dutch variant of RM4SCC https://en.wikipedia.org/wiki/RM4SCC) for your generator. It's a bit different than the other codes you support, so I had to add a different way of drawing bars in the barcode drawing functions in the svg and canvas renderers.

Could you have a look and check if this would be something you would pull? If so, I can also add the RM4SCC and correct validation for KIX (not needed for my own use): https://github.com/abmaonline/JsBarcode/tree/kix_support

lindell commented 7 years ago

I would absolutely like to merge KIX support but the way you have implemented it is not something I would merge. The code does not follow SOLID, namely DIP (Dependency Inversion Principle). Take a look at this code I have been thinking about an extension to the internal API that would allow for these kind of barcodes, like PostBar and POSTNET, but have not decided on the change yet.

One implementation that could work for all these barcode and still be backwards compatible would be:

[
{"start": 0, "end": 0.5},
0,
{"start": 0.5, "end": 0.75},
0,
{"start": 0.5, "end": 1},
]

Which would result in:

abmaonline commented 7 years ago

You're completely right! That was also the reason for my question, since I could not find any extension points for codes that are not strictly 1D. So great to hear you have already been thinking on a solution for this issue 👍

Maybe another idea to implement vertical info and make it ready for a lot of 2D codes at the same time, is to not only specify a horizontal resolution, but also a vertical resolution. That way it is possible to render most 1D and 2D codes. For 1.5D codes like KIX it would be a little overkill (probably 3 + 2 + 3 = 8 bits per bar for the right proportions of the bars), but for 2D codes like QR it would be a 1:1 match.

Your example would be encoded something like this: [ "1100", "0000", "0010", "0000", "0011" ]

lindell commented 7 years ago

That is actually one implementation I have not thought about but think is really interesting. Yours and the one I proposed does both have advantages and disadvantages. Yours will allow for multiple lines on the same x-axis value, but does not allow for completely free setting of the values. I will take a look at more barcodes and see what would be best. (It should be possible to have both but unnecessary if it can be avoided)

lindell commented 7 years ago

It might be that I rewrite the entire internal API for the barcodes. Using strings is a remnant from the first days of this library and it seems like removing it all together for arrays would be a good idea. (Even if it is almost identical implementation wise.)

abmaonline commented 7 years ago

Since all bit arrays/strings are generate in the code, arrays are probably more intuitive (I like strings for input however, a simple .split('') saves a lot of quotes and comma's 😉)

For > 1D There seem to be two cases indeed: 1) vertical resolution much lower than horizontal (only 2 or 3 options: Postnet, USPS4CB, RM4SCC, KIX, Postbar, etc) 2) vertical resolution (almost) equal to horizontal resolution (most 2D codes)

Weird case: PDF417 😋

First case could work with the start-end object (by providing an array of them it is even possible to use multiple bars in one line). Maybe use the info in the object as a sort of mask for the encoded bits, since most codes only have discrete values and this makes the encoding step much easier (and in the renderer you have to do all kinds of visual calculations anyway). So in case of KIX specify [0.37, 0.26, 0.37] in the options for the renderer and use "110" for a high bar (011 = low bar, 111 = full bar, 010 = only sync). But maybe this is not generic enough...

Second case is quite trivial with a 2D array with bits.

Not sure if first case is needed, but would make the postal codes easier 😁

lindell commented 7 years ago

2D barcodes are out of the scope of this library. It will only focus on 1D/1.5D barcodes.

I will implement my suggestion to start with since that will allow for all Postnet, USPS4CB, RM4SCC, KIX, Postbar, etc, barcodes to be implemented with a general case change in the internal API.

lindell commented 7 years ago

If you want to implement the barcodes with this you could pull from https://github.com/lindell/JsBarcode/tree/internal-api-extension It is not finalized but could be useful to ensure that the implementation works :)

abmaonline commented 7 years ago

Sounds good! Let me see if I can implement KIX using your branch when I have some time 👍

abmaonline commented 7 years ago

Seems to work fine: https://github.com/abmaonline/JsBarcode/tree/kix-support-with-api-extension

lindell commented 7 years ago

Your implementation contained solutions that where a little bit too convoluted, I cloned your code and changed it a bit to basically be 7 lines + definitions.

https://github.com/lindell/JsBarcode/blob/kix-support-with-api-extension/src/barcodes/KIX/index.js

abmaonline commented 7 years ago

Looks good!

Is this the way you want to keep the structure for 1.5D codes? If so. I can add some of the other postal barcodes. Is it ok if I use the setup from the MSI codes for a postal base class with extensions for the various implementations?

lindell commented 7 years ago

The structure should be the most fitting for the barcode type. Thought this was the best solution. I would love to have more barcodes implemented if you want to take the time :+1: And the way the MSI barcodes are implemented is preferred. If code can be reused from inheritance that's the best 😄

lindell commented 7 years ago

Made the sync height (default: 0.37) as an option and an option (default: true) to make the height of the lines integers in case when the height is going to be an uneven number.

JsBarcode("1234", {format: "KIX", width: 5, height: 50, integerHeight: false})

JsBarcode("1234", {format: "KIX", width: 5, height: 50, integerHeight: true})

lindell commented 7 years ago

I'm thinking about merging into master. Do you have anything else you want to change about KIX before that?

abmaonline commented 7 years ago

Just created a group for the postal codes, created a base3 base class (read in one of the articles on this encoding the encoding using this type of bar is called this), added the RM4SCC start/stop chars to it, so it is possible to encode any data using this type (it's what I need it for 😇).

Created a separate KIX class with the specific KIX validation (Dutch postal code + house number + optional extension). Added some tests for this.

Didn't add RM4SCC yet, since the checksum is a bit tricky.

https://github.com/abmaonline/JsBarcode/commit/b4a8cf9b5b08b6d7b9c1b1a95bd064cff6ec925c