From: dswitkin Date: Tue, 18 Nov 2008 15:34:45 +0000 (+0000) Subject: My BitVector implementation was totally buggy. I rewrote a lot of it and ported the... X-Git-Url: http://git.rot13.org/?a=commitdiff_plain;h=80f6fb2233996a8135a6051499c16c2ff90bff3f;p=zxing.git My BitVector implementation was totally buggy. I rewrote a lot of it and ported the test case, which now passes. Now I'm tracking down an assertion in Encoder.InterleaveWithECBytes(). git-svn-id: http://zxing.googlecode.com/svn/trunk@717 59b500cc-1b3d-0410-9834-0bbf25fbcc57 --- diff --git a/core/src/com/google/zxing/qrcode/encoder/BitVector.java b/core/src/com/google/zxing/qrcode/encoder/BitVector.java index 0a76864f..c2475318 100644 --- a/core/src/com/google/zxing/qrcode/encoder/BitVector.java +++ b/core/src/com/google/zxing/qrcode/encoder/BitVector.java @@ -27,24 +27,22 @@ package com.google.zxing.qrcode.encoder; public final class BitVector { private int sizeInBits; - private int bytePosition; private byte[] array; // For efficiency, start out with some room to work. - private static final int DEFAULT_SIZE_IN_BITS = 32 * 8; + private static final int DEFAULT_SIZE_IN_BYTES = 32; public BitVector() { - sizeInBits = DEFAULT_SIZE_IN_BITS; - bytePosition = 0; - array = new byte[DEFAULT_SIZE_IN_BITS / 8]; + sizeInBits = 0; + array = new byte[DEFAULT_SIZE_IN_BYTES]; } // Return the bit value at "index". public int at(final int index) { Debug.DCHECK_LE(0, index); Debug.DCHECK_LT(index, sizeInBits); - final int value = array[index / 8]; - return (value >> (7 - (index % 8))) & 1; + final int value = array[index >> 3] & 0xff; + return (value >> (7 - (index & 0x7))) & 1; } // Return the number of bits in the bit vector. @@ -53,20 +51,23 @@ public final class BitVector { } // Return the number of bytes in the bit vector. + // JAVAPORT: I would have made this ((sizeInBits + 7) >> 3), but apparently users of this class + // depend on the number of bytes being rounded down. I don't see how that works though. public int num_bytes() { - return sizeInBits / 8; + return sizeInBits >> 3; } // Append one bit to the bit vector. public void AppendBit(final int bit) { Debug.DCHECK(bit == 0 || bit == 1); - final int num_bits_in_last_byte = sizeInBits % 8; + final int num_bits_in_last_byte = sizeInBits & 0x7; // We'll expand array if we don't have bits in the last byte. if (num_bits_in_last_byte == 0) { appendByte(0); + sizeInBits -= 8; } // Modify the last byte. - array[array.length - 1] |= (bit << (7 - num_bits_in_last_byte)); + array[sizeInBits >> 3] |= (bit << (7 - num_bits_in_last_byte)); ++sizeInBits; } @@ -82,10 +83,9 @@ public final class BitVector { int num_bits_left = num_bits; while (num_bits_left > 0) { // Optimization for byte-oriented appending. - if (sizeInBits % 8 == 0 && num_bits_left >= 8) { + if ((sizeInBits & 0x7) == 0 && num_bits_left >= 8) { final int newByte = (value >> (num_bits_left - 8)) & 0xff; appendByte(newByte); - sizeInBits += 8; num_bits_left -= 8; } else { final int bit = (value >> (num_bits_left - 1)) & 1; @@ -106,7 +106,8 @@ public final class BitVector { // Modify the bit vector by XOR'ing with "other" public void XOR(final BitVector other) { Debug.DCHECK_EQ(sizeInBits, other.size()); - for (int i = 0; i < array.length; ++i) { + int sizeInBytes = (sizeInBits + 7) >> 3; + for (int i = 0; i < sizeInBytes; ++i) { // The last byte could be incomplete (i.e. not have 8 bits in // it) but there is no problem since 0 XOR 0 == 0. array[i] ^= other.array[i]; @@ -128,16 +129,22 @@ public final class BitVector { return result.toString(); } + // Callers should not assume that array.length is the exact number of bytes needed to hold + // sizeInBits - it will typically be larger for efficiency. + public byte[] getArray() { + return array; + } + // Add a new byte to the end, possibly reallocating and doubling the size of the array if we've // run out of room. private void appendByte(int value) { - if (bytePosition >= array.length) { + if ((sizeInBits >> 3) == array.length) { byte[] newArray = new byte[array.length * 2]; System.arraycopy(array, 0, newArray, 0, array.length); array = newArray; } - array[bytePosition] = (byte) value; - bytePosition++; + array[sizeInBits >> 3] = (byte) value; + sizeInBits += 8; } } diff --git a/core/src/com/google/zxing/qrcode/encoder/Debug.java b/core/src/com/google/zxing/qrcode/encoder/Debug.java index 7ce5416a..69774603 100644 --- a/core/src/com/google/zxing/qrcode/encoder/Debug.java +++ b/core/src/com/google/zxing/qrcode/encoder/Debug.java @@ -25,11 +25,11 @@ package com.google.zxing.qrcode.encoder; public class Debug { public static void LOG_ERROR(String message) { - // TODO: Implement + throw new IllegalStateException(message); } public static void LOG_INFO(String message) { - // TODO: Implement + throw new IllegalStateException(message); } public static void DCHECK(boolean condition) { diff --git a/core/src/com/google/zxing/qrcode/encoder/Encoder.java b/core/src/com/google/zxing/qrcode/encoder/Encoder.java index 16109419..9525cb83 100644 --- a/core/src/com/google/zxing/qrcode/encoder/Encoder.java +++ b/core/src/com/google/zxing/qrcode/encoder/Encoder.java @@ -613,7 +613,7 @@ private static final ECPolyInfo kECPolynomials[] = { return true; } Debug.LOG_ERROR("Interleaving error: " + num_total_bytes + " and " + result.num_bytes() + - "differ."); + " differ."); return false; } diff --git a/core/test/src/com/google/zxing/qrcode/encoder/BitVectorTestCase.java b/core/test/src/com/google/zxing/qrcode/encoder/BitVectorTestCase.java new file mode 100644 index 00000000..06397881 --- /dev/null +++ b/core/test/src/com/google/zxing/qrcode/encoder/BitVectorTestCase.java @@ -0,0 +1,173 @@ +/* + * Copyright 2008 ZXing authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.zxing.qrcode.encoder; + +import junit.framework.TestCase; + +/** + * @author satorux@google.com (Satoru Takabayashi) - creator + * @author dswitkin@google.com (Daniel Switkin) - ported from C++ + */ +public class BitVectorTestCase extends TestCase { + + private static int getUnsignedByte(BitVector v, int index) { + return v.getArray()[index] & 0xff; + } + + public void testAppendBit() { + BitVector v = new BitVector(); + assertEquals(0, v.num_bytes()); + // 1 + v.AppendBit(1); + assertEquals(1, v.size()); + assertEquals(0x80, getUnsignedByte(v, 0)); + // 10 + v.AppendBit(0); + assertEquals(2, v.size()); + assertEquals(0x80, getUnsignedByte(v, 0)); + // 101 + v.AppendBit(1); + assertEquals(3, v.size()); + assertEquals(0xa0, getUnsignedByte(v, 0)); + // 1010 + v.AppendBit(0); + assertEquals(4, v.size()); + assertEquals(0xa0, getUnsignedByte(v, 0)); + // 10101 + v.AppendBit(1); + assertEquals(5, v.size()); + assertEquals(0xa8, getUnsignedByte(v, 0)); + // 101010 + v.AppendBit(0); + assertEquals(6, v.size()); + assertEquals(0xa8, getUnsignedByte(v, 0)); + // 1010101 + v.AppendBit(1); + assertEquals(7, v.size()); + assertEquals(0xaa, getUnsignedByte(v, 0)); + // 10101010 + v.AppendBit(0); + assertEquals(8, v.size()); + assertEquals(0xaa, getUnsignedByte(v, 0)); + // 10101010 1 + v.AppendBit(1); + assertEquals(9, v.size()); + assertEquals(0xaa, getUnsignedByte(v, 0)); + assertEquals(0x80, getUnsignedByte(v, 1)); + // 10101010 10 + v.AppendBit(0); + assertEquals(10, v.size()); + assertEquals(0xaa, getUnsignedByte(v, 0)); + assertEquals(0x80, getUnsignedByte(v, 1)); + } + + public void testAppendBits() { + { + BitVector v = new BitVector(); + v.AppendBits(0x1, 1); + assertEquals(1, v.size()); + assertEquals(0x80, getUnsignedByte(v, 0)); + } + { + BitVector v = new BitVector(); + v.AppendBits(0xff, 8); + assertEquals(8, v.size()); + assertEquals(0xff, getUnsignedByte(v, 0)); + } + { + BitVector v = new BitVector(); + v.AppendBits(0xff7, 12); + assertEquals(12, v.size()); + assertEquals(0xff, getUnsignedByte(v, 0)); + assertEquals(0x70, getUnsignedByte(v, 1)); + } + } + + public void testNumBytes() { + BitVector v = new BitVector(); + assertEquals(0, v.num_bytes()); + v.AppendBit(0); + assertEquals(0, v.num_bytes()); + v.AppendBits(0, 7); + assertEquals(1, v.num_bytes()); + v.AppendBits(0, 8); + assertEquals(2, v.num_bytes()); + } + + public void testAppendBitVector() { + BitVector v1 = new BitVector(); + v1.AppendBits(0xbe, 8); + BitVector v2 = new BitVector(); + v2.AppendBits(0xef, 8); + v1.AppendBitVector(v2); + // beef = 1011 1110 1110 1111 + assertEquals("1011111011101111", v1.toString()); + } + + public void testXOR() { + { + BitVector v1 = new BitVector(); + v1.AppendBits(0x5555aaaa, 32); + BitVector v2 = new BitVector(); + v2.AppendBits(0xaaaa5555, 32); + v1.XOR(v2); + assertEquals(0xff, getUnsignedByte(v1, 0)); + assertEquals(0xff, getUnsignedByte(v1, 1)); + assertEquals(0xff, getUnsignedByte(v1, 2)); + assertEquals(0xff, getUnsignedByte(v1, 3)); + } + { + BitVector v1 = new BitVector(); + v1.AppendBits(0x2a, 7); // 010 1010 + BitVector v2 = new BitVector(); + v2.AppendBits(0x55, 7); // 101 0101 + v1.XOR(v2); + assertEquals(0xfe, getUnsignedByte(v1, 0)); // 1111 1110 + } + } + + public void testAt() { + BitVector v = new BitVector(); + v.AppendBits(0xdead, 16); // 1101 1110 1010 1101 + assertEquals(1, v.at(0)); + assertEquals(1, v.at(1)); + assertEquals(0, v.at(2)); + assertEquals(1, v.at(3)); + + assertEquals(1, v.at(4)); + assertEquals(1, v.at(5)); + assertEquals(1, v.at(6)); + assertEquals(0, v.at(7)); + + assertEquals(1, v.at(8)); + assertEquals(0, v.at(9)); + assertEquals(1, v.at(10)); + assertEquals(0, v.at(11)); + + assertEquals(1, v.at(12)); + assertEquals(1, v.at(13)); + assertEquals(0, v.at(14)); + assertEquals(1, v.at(15)); + } + + public void testToString() { + BitVector v = new BitVector(); + v.AppendBits(0xdead, 16); // 1101 1110 1010 1101 + assertEquals("1101111010101101", v.toString()); + } + +}