DDMAL / Rodan

:dragon_face: A web-based workflow engine.
https://rodan2.simssa.ca/
47 stars 13 forks source link

Refactoring of RodanKit JS #179

Closed mrbannon closed 11 years ago

mrbannon commented 11 years ago

The RodanKit resources should get a once-over, and some refactoring should be done (e.g. binarisation JS).

ahankinson commented 11 years ago

We've started establishing a RodanKit JS module convention, so I'd like to see the code re-factored to follow this. See RKBinarise.js and RKSegment.js for the pattern.

I would like to see the following happen:

-- The crop interface and crop.js replaced with RKSegment.js, potentially with an option to "lock sides" so that moving a point resizes the sides, not just moves a point. (i.e., behaves more like a cropping interface than a polygon selection interface).

-- Refactor the existing interfaces: rotate.js, despeckle.js. They should follow the following template (replacing RKTemplate with RKRotate or RKDespeckle as needed)

(function ($)
{
    var RKTemplate = function(element, options)
    {
        var defaults = {
             /// user settings go here
             settingOne: null,
             settingTwo: null
        };

        var settings = $.extend({}, defaults, options);

        var globals = {
              // "class" variables go here
              initialFoo: null,
              initialBar: null
        };

        $.extend(settings, globals);

        var init = function()
        {
            // class init function
        };

    init();

    var privateFunction = function(arg1, arg2)
    {
        // private function code
    };

    this.publicFunction = function(arg1, arg2)
    {
         // public function stuff
    };
    };
    $.fn.RKTemplate = function(options)
    {
        return this.each(function ()
        {
            var element = $(this);

            if (element.data('RKTemplate'))
                return;

            options.parentObject = element[0];

            var template = new RKTemplate(this, options);
            element.data('RKTemplate', template);
            });
        };
})(jQuery);

This is based on how our diva.js code is laid out. It may then be initialized like this:

<script type="text/javascript">
    $(document).ready(function() {
        $('#template-wrapper').RKTemplate({
                settingOne: "foo",
                settingTwo: "bar"
        });
        var tp = $('#template-wrapper').data('RKTemplate');
    });
</script>

This stores the object instance in the data field on the element, so that you can retrieve the instance by grabbing the wrapper element (var tp = $('#template-wrapper').data('RKTemplate');). This means you can call public methods by then doing this: tp.publicFunction(arg1, arg2)

For this task you shouldn't need to replace the logic, just re-factor the code. There are a few places I have been doing some micro-optimizations:

-- If a for loop can be replaced with a reverse while loop, that might save us a few cycles. This is esp. important if you're cycling through all the pixels in an image. You might have a look at the results here: http://jsperf.com/loops/70 and choose one that seems to work in most cases. If you come across a do ... while loop, definitely replace it. Also, if you come across for (x in obj) you should figure out a way to replace that.

-- An important optimization we've found is that you don't have to do pixel value checking, since Canvas will constrain the values automatically. So you don't have to do:

if (value > 255)
{
    value = 255;
}
else if (value < 0)
{
    value = 0;
}

You can just blindly assign values above and below the range and canvas will take care of it.

-- Reformatting to follow the Allman-style brackets that we use in the rest of the project.

-- If you can avoid blindly using the jQuery selectors and using the straight-up JS stuff, that might also help speed things up. For example, you can replace $('#some-id') with document.getElementById('some-id'). If you need a jQuery object you can optimize the selector by doing this: $(document.getElementById('some-id')); See these for reasons and methods of doing this.

lbaribeau commented 11 years ago

Status:

Crop: RKSegment has been used to create a crop interface (thanks to Ryan). The option to 'lock sides' isn't done.
Rotate: Completely done. Despeckle: Has been refactored but just needs to be integrated into Rodan (a job created for it).

lbaribeau commented 11 years ago

Created issue 276 to handle remaining despeckle task and closing.