Opened 8 years ago

Closed 7 years ago

#14007 closed enhancement (fixed)

[patch][ccla]Add SVG rendering options to GFX

Reported by: cjolif Owned by: Patrick Ruzand
Priority: high Milestone: 1.8
Component: DojoX GFX Version: 1.6.1
Keywords: Cc: Patrick Ruzand
Blocked By: Blocking:

Description (last modified by Patrick Ruzand)

SVG provides two properties shape-rendering and text-rendering to allow user to specify the antialiasing behavior, they should be exposed at GFX level.

Attachments (3)

14007.patch (3.4 KB) - added by Patrick Ruzand 7 years ago.
patch by pruzand (IBM, CCLA)
14007.2.patch (3.2 KB) - added by Patrick Ruzand 7 years ago.
updated to match last discussion
14007.3.patch (3.2 KB) - added by Patrick Ruzand 7 years ago.
clean up patch.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by cjolif

Component: GeneralDojoX GFX
Owner: set to Eugene Lazutkin

comment:2 Changed 7 years ago by Patrick Ruzand

Owner: changed from Eugene Lazutkin to Patrick Ruzand
Status: newassigned

comment:3 Changed 7 years ago by Patrick Ruzand

Description: modified (diff)
Summary: Add antialiasing options to GFXAdd SVG rendering options to GFX

comment:4 Changed 7 years ago by Patrick Ruzand

Note: Since it is a svg-specific feature (other renderer do not offer such setting), it makes sense to me to provide this new capability in an optional module ("dojox/gfx/svgext" for ex) that would be explicitly required by the dev (such renderer-specific extension module is the purpose of http://trac.dojotoolkit.org/ticket/11232 ).

As I see it, a possible approach would be to extend the current svg.Shape definition to add the required api in the svgext module. What the api could be depends on what kind of support we want to provide. SVG defines the following rendering hint that can be set on a shape: color-rendering, shape-rendering, text-rendering (text only), image-rendering (image only). If we want to support all of them in a transparent way with a single api, then the name should be generic enough, for ex:

setRenderingOption: function(...)

Regarding the parameter(s), it could be either:

  1. if we don't want to bother with option combination: function(string, string): 1st param is the option name (e.g. "shape-rendering"), the 2nd the value (e.g. "optimizeSpeed").
  2. if we want to allow option combination: function(object) where "object" is an hash containing the values of the different options to set (e.g. {"shape-rendering":"optimizeSpeed", "color-rendering":"optimizeQuality"} )
  3. both: function(object | string, string?)

The last choice has my personal preference.

Note regarding the parameter-as-object : since the rendering option names are not valid javascript property identifiers, the user must write the option name using quote when writing the option parameter: { "shape-rendering" : "optimizeSpeed" }. Should we support an alternate js-friendly naming (e.g shape-rendering -> shapeRendering, etc.) too, to make it easier ? IMO, it does not worth it (since we are in a svg-specific context, let's stick to the SVG naming).

I have attached a patch that implements the (c) proposition.

Changed 7 years ago by Patrick Ruzand

Attachment: 14007.patch added

patch by pruzand (IBM, CCLA)

comment:5 Changed 7 years ago by Patrick Ruzand

Summary: Add SVG rendering options to GFX[patch][ccla]Add SVG rendering options to GFX

comment:6 Changed 7 years ago by cjolif

Overall I agree with your solution. Having it as separated module is probably a good thing. Two small remarks:

1/ I think (a) solution is simpler and probably enough. That said in this case shouldn't it be named addRenderingOption more than setRenderingOption(s?)?

2/ We should think of how to document that in terms of API doc. I like the idea of having an optional module but then someone looking at Shape API won't see it? Is there a way to get it displayed in Shape API with a message like "SVG only"?

comment:7 Changed 7 years ago by Patrick Ruzand

1/ I think (a) solution is simpler and probably enough. That said in this case shouldn't it be named addRenderingOption more than setRenderingOption(s?)?

in this case, addRenderingOption is indeed better, yes.

2/ We should think of how to document that in terms of API doc. I like the idea of having an optional module but then someone looking at Shape API won't see it? Is there a way to get it displayed in Shape API with a message like "SVG only"?

cannot say. I will investigate what's feasible with the doc tool.

I will rework the patch to match the new naming.

Changed 7 years ago by Patrick Ruzand

Attachment: 14007.2.patch added

updated to match last discussion

comment:8 Changed 7 years ago by Patrick Ruzand

A comment regarding the svgext module: because there are other svg-specific tickets that may need to be put in an independent module, I will keep the packaging of this patch (ie in a svgext.js module) as it is, even if there's only very few lines of code. If it happens that there's eventually no need for such independent extension, we could easily migrate this patch to svg.js.

Changed 7 years ago by Patrick Ruzand

Attachment: 14007.3.patch added

clean up patch.

comment:9 Changed 7 years ago by Patrick Ruzand

Resolution: fixed
Status: assignedclosed

In [28259]:

initial revision, fixes #14007

Note: See TracTickets for help on using tickets.