One general note:
The Uint stuff is one thing of this API I hate very much in the context
of GWT. There are two solutions:
1. use emulated longs, and somehow convert them to real primitive JS
numbers (either using a string, as we did in some places or using
doubles behind the scene)
2. use double.
I thought about this again and now I think using long is always a bad
idea. As you already said, double is the best match for that in GWT.
So the real question here is: Shoud we make it easy for the user to use
long values with that API or is it better to force the usage of double?
If we'd provide a "decent" conversion method for long[]->JsArrayNumber,
the user can call it manually and use that JsArray with the TypedArrays
if he explicitly want's to do so. For the single value setters one can
simply cast long->double.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/DataView.java
File user/src/com/google/gwt/typedarrays/client/DataView.java (right):
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/DataView.java#newcode128
user/src/com/google/gwt/typedarrays/client/DataView.java:128: private
static native boolean isSupportedRuntime() /*-{
On 2012/01/01 22:34:57, tbroyer wrote:
Is there really a need for a distinct check besides
TypedArray.isSupported()?
AFAICT, only Chrome m7 and m8 supported "typed arrays" without
supporting
DataView, and those versions are no longer used on the web; or are
there mobile
browsers out there with similar "partial support"?
Yes, it's necessary. Mozilla is a little bit late with that
implementation:
https://bugzilla.mozilla.org/show_bug.cgi?id=575688
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/Int16Array.java
File user/src/com/google/gwt/typedarrays/client/Int16Array.java (right):
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/Int16Array.java#newcode24
user/src/com/google/gwt/typedarrays/client/Int16Array.java:24: public
class Int16Array extends IntBasedTypedArray<Int16Array> {
On 2012/01/01 22:34:57, tbroyer wrote:
I'm not sure the IntBasedTypedArray intermediate type is worth it, at
least for
reading (the TypedArrays spec defines how values are converted, so you
can
technically put a 'double' into an Int16Array; but for reading, I
think the
getter return value should be of the type of the stored value: i.e.
'short' for
an Int16Array).
What do you think about that:
- adding setters for int, float, double and byte[], short[], int[],
float[], double[], JsArrayInteger, JsArrayNumber to TypedArray. As you
said, the values are automatically converted. so the user has all
options to set values.
- adding getters to the concrete Arrays using the matching Java types
for the IntXX and FloatXX Arrays and Unit8->short, Uint16->int,
Unit32->double
- Let's use that type mapping for the create methods too. If you have an
array of a different type you can manually call the JsArrayUtil and use
the resulting JsArray with the factory method.
=> Then we can remove the IntBasedTypedArray as you said without to much
copy/paste code.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/Int16Array.java#newcode113
user/src/com/google/gwt/typedarrays/client/Int16Array.java:113: public
static Int16Array create(int[] array) {
On 2012/01/01 22:34:57, tbroyer wrote:
Shouldn't it rather be short[] ? or at least have a short[] overload?
Yes, see the comment above for the detailed proposal.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/IntBasedTypedArray.java
File user/src/com/google/gwt/typedarrays/client/IntBasedTypedArray.java
(right):
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/IntBasedTypedArray.java#newcode176
user/src/com/google/gwt/typedarrays/client/IntBasedTypedArray.java:176:
public final void set(int[] array) {
On 2012/01/01 22:34:57, tbroyer wrote:
How about using varargs here? Could make things easier for a Java
developer:
set(1, 2, 3) instead of set(new int[] { 1, é, 3 })
Yes, good idea.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java
File user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java
(right):
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java#newcode56
user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java:56: public
static native String getStringFromJsArrayInteger(JsArrayInteger array,
int index) /*-{
On 2012/01/01 22:34:57, tbroyer wrote:
Is that used somewhere else that getLongFromJsArrayInteger above?
(i.e. does it
need to be 'public'? could it be removed if getLongFromJsArrayInteger
were
reworked around double-to-long casts?)
Yes, it could be removed.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java#newcode87
user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java:87: *
Converts a JsArrayString to a String[].
On 2012/01/01 22:34:57, tbroyer wrote:
Javadoc should make it *very* clear that it makes a copy in dev mode
but a
"cast" in prod mode, so the returned array should only be used for
reading, and
the array passed as argument should also only be used for reading
while the
returned array is in use: any change to one of the arrays would induce
different
behaviors in dev mode vs. prod mode.
That means the method name ("unwrapArray") does not accurately
describes its
behavior.
In my own code, I call these "bridge" methods "forIteration" because
that's
basically all they should be used for.
At the beginning, we only used these methods for dev mode and used a
if(GWT.isScript()) at all places where arrays are used. That's the
reason why they have unsuitable names.
What do you think about using the names "asJsArray" and "asJavaArray"?
That's a neutral description of what they do.
And yes, you are right, the JavaDoc should be clear about the side
effect.
http://gwt-code-reviews.appspot.com/1621803/diff/1/user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java#newcode114
user/src/com/google/gwt/typedarrays/client/JsArrayUtil.java:114: for
(int i = 0; i < srcArray.length; i++) {
On 2012/01/01 22:34:57, tbroyer wrote:
How about a "for each" loop w/ JsArrayBoolean.push()? I doubt it'd
change
anything performance-wise but it would make the code slightly more
readable and
less error-prone:
for (boolean b : srcArray) {
result.push(b);
}
Good idea.
http://gwt-code-reviews.appspot.com/1621803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
