Bug 69427 - HTML canvas strokes with dash and dashOffset for V8.
Summary: HTML canvas strokes with dash and dashOffset for V8.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-05 08:06 PDT by Young Han Lee
Modified: 2013-05-02 11:13 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.49 KB, patch)
2011-10-05 08:10 PDT, Young Han Lee
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Young Han Lee 2011-10-05 08:06:41 PDT
HTML canvas strokes with dash and dashOffset for V8.
Comment 1 Young Han Lee 2011-10-05 08:10:44 PDT
Created attachment 109794 [details]
Patch
Comment 2 Young Han Lee 2011-10-05 08:18:29 PDT
This is a follow-up patch for the bug 63933.
Comment 3 Adam Barth 2011-10-18 10:57:46 PDT
Comment on attachment 109794 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109794&action=review

> Source/WebCore/bindings/v8/custom/V8CanvasRenderingContext2DCustom.cpp:116
> +    v8::Local<v8::Array> result = v8::Array::New(dash->size());
> +    DashArray::const_iterator end = dash->end();
> +    int index = 0;
> +    for (DashArray::const_iterator it = dash->begin(); it != end; ++it, ++index)
> +        result->Set(v8::Integer::New(index), v8::Number::New(*it));
> +
> +    return result;
> +}

I think we already have a function that does this.  If we don't have one, you should add it.  It's a very general operation.

Also, per the recent webkit-dev thread, we should be using indicies rather than iterators to walk arrays.  That will also help you avoid the nuttiness of incrementing the iterator and the index at the same time.
Comment 4 Young Han Lee 2011-10-23 03:15:13 PDT
> 
> I think we already have a function that does this.  If we don't have one, you should add it.  It's a very general operation.

To clarify what 'a function that does this' means...

1. Do you mean a function converting Vector<float> to v8:Array?
(e.g. v8::Handle<v8::Value> toV8(const Vector<float>& vector))

This is not a general operation. The patch I uploaded is the only place using the operation, so I don't think we should add the function.


2. Do you mean a function converting any Vector to v8::Array?
(e.g. template <class T> v8::Handle<v8::Value> toV8(const T& vector))

I'm not sure it is possible to generalize this kind of operation.

There are only two places doing this operation under WebCore/bindings/v8, and following is one of them.

typedef Vector<RefPtr<ScriptProfile> > ProfilesArray;

v8::Handle<v8::Value> V8Console::profilesAccessorGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
{
    ......
    const ProfilesArray& profiles = imp->profiles();
    ......

    for (ProfilesArray::const_iterator iter = profiles.begin(); iter != end; ++iter)
        result->Set(v8::Integer::New(index++), toV8(iter->get()));
    return result;
}

This function converts the element of the Vector like this 'toV8(iter->get())', because the element is RefPtr. But my patch converts like this 'v8::Number::New(*iter))', because the element is just float.

I don't know a clear way to generalize these two function. Any Idea? 


> 
> Also, per the recent webkit-dev thread, we should be using indicies rather than iterators to walk arrays.  That will also help you avoid the nuttiness of incrementing the iterator and the index at the same time.

agree.
Comment 5 Anders Carlsson 2013-05-02 11:13:11 PDT
V8 is gone from WebKit.