Suboptimal segements chosen, causing bitmap generation to fail

[awbacker]

I use the library heavily to generate qr bitmaps (manually creating the image), so thank you for creating it!

The following test case should generate correctly:

• AlphaNumeric (single segment)
• V4
• Ecc H (Highest)
• Changing 6998877 to 699E877 causes generation to succeed

func TestShouldCreateTheQrCode(t *testing.T) {
	msg := "HTTPS://ABC.DE/F/393AABB6998877XYZ0518AUQCRVJN25"
	//                                 E    <-- change to E and no crash
	qrc, err := qrcode.NewWithForcedVersion(msg, 4, qrcode.Highest)
	if err != nil {
		t.Error(err)
	}
	qrc.Bitmap()
}

=== RUN   TestShouldCreateTheQrCode
2020/06/17 12:26:01 BUG: got len 296, expected 288
--- FAIL: TestShouldCreateTheQrCode (0.00s)
panic: BUG: got len 296, expected 288 [recovered]
	panic: BUG: got len 296, expected 288

I noticed that after the call to encoder.encode the length is 290 (and somehow numPaddingBits() == -2?). In any case, I am not familiar enough with the code to debug fully yet, but will be giving it a try this afternoon.

The multi-segments by default kind of thing is clever, it just appears to be failing in this case. I feel that most users will calculate their version/recovery-level based on the published data lengths which assume single-segment, especially for high volumes.

[skip2]

Hi Andrew.

Thanks for the kind words.

You're right, the optimiser is being too clever, and a single alphanumeric segment would be a more efficient (and more predictable) encoding.

We currently use a single byte segment when it's more efficient than multiple segments. I've now added use of a single alphanumeric segment (only if the content allows it), when it's more efficient than multiple segments.

da1b656

let me know if that helps,

thanks,

skip2

[awbacker]

I took a look at that and it seems like it would work, but I haven't had a chance to try it yet. Looking through your code taught me a lot about QR codes, even though I already knew enough to do the basic stuff like determine a good ecc. Well done :)

In the end I made below change in my vendored version.

// rather than try only binary, try the "best" encoding based on the data as a 
// single segment in encode([]bytes). 
minEncoding := minDataEncoding(d.data)
singleSegment, err := d.encodedLength(minEncoding, len(d.data))
...

func minDataEncoding(data []byte) dataMode {
	m := dataModeNone
	for _, v := range data {
		if n := minByteEncoding(v); n > m {
			m = n
		}
		if m == dataModeByte {
			break
		}
	}
	return m
}

// smallest mode that applies to a single byte
func minByteEncoding(chr byte) dataMode {
	if chr >= '0' && chr <= '9' {
		return dataModeNumeric
	}
	if chr >= 'A' && chr <= 'Z' {
		return dataModeAlphanumeric
	}
	switch chr {
	case ':', '/', '.', '-', '+', '*', '$', '%':
		return dataModeAlphanumeric
	}
	return dataModeByte
}

I may fork this repo and create a PR, if thats helpful to you in some way. I'll and add this change, and possibly others to make writing "your own" NewWithFixedVersion analog easier, such as public functions for "utility" work, and public functions, and possibly decomposing encode a bit.

My particular case, generating an insane amount of qr codes:

• All data is identical in length
• All data is either alphanum or binary, 99% alphanum
• Version is fixed
• Ecc has a min value, but > that is fine
• Speed is important
• No image generation/etc (I do that manually from the bitmap)

Anyway, the code has been wonderful to work with. Its easy for me to put something together that does just the 1 thing I want, but you have so much more in there! Feel free to close this, as you see fit.

[fenglijunnb]

@awbacker
Hi Andrew.
I really appreciate your PR, but I have a question about how to generate the flower-style QR code. I really don't have a clue. I hope you can help me

[fenglijunnb]

@awbacker This is a similar image that I downloaded from some platforms