Lines with zero length dashes not drawn

RESOLVED FIXED in mozilla37

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: j.tosovsky, Assigned: longsonr)

Tracking

({regression})

35 Branch
mozilla37
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141229214612

Steps to reproduce:

Open http://drifted.in/other/dash_array.svg


Actual results:

There are missing dots in the image. 


Expected results:

When round linecap is set, there IMO should be dots visible even for zero length dashes. It is a regression. It worked in previous versions - at least in 2013 when I added my comment to the similar Chrome issue: https://code.google.com/p/chromium/issues/detail?id=162424
I see a line on Firefox 34 but not on trunk. A regression range would help. If you want to assist with that Jan, then there's a tool to assist with that http://mozilla.github.io/mozregression/
Status: UNCONFIRMED → NEW
Ever confirmed: true
Could be bug 1075399 since that landed for Firefox 35.
Assignee: nobody → longsonr
Posted patch round.txt (obsolete) — Splinter Review
Attachment #8543652 - Flags: review?(jwatt)
Posted patch round.txt (obsolete) — Splinter Review
Attachment #8543652 - Attachment is obsolete: true
Attachment #8543652 - Flags: review?(jwatt)
Attachment #8543708 - Flags: review?(jwatt)
Regression range:

--- (m-c):
Last good revision: 7c24470b6b3a (2014-09-30)
First bad revision: 14665b1de5ee (2014-10-01)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7c24470b6b3a&tochange=14665b1de5ee 

---(m-i):
Last good revision: 3b427c2ee147
First bad revision: 0ade54570f25
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3b427c2ee147&tochange=0ade54570f25 

Also reproducible on Nightly 37.0a1 (Build ID: 20150104030205) - Mac OS X 10.9.5. and Ubuntu 14.04 32bit
OS: Windows 7 → All
Hardware: x86_64 → All
Thanks for the range Alexandra. The regression is from https://hg.mozilla.org/integration/mozilla-inbound/rev/cd87b4cb26c1
Blocks: 1074294
Comment on attachment 8543708 [details] [diff] [review]
round.txt

>+  // If the sum of the values is zero,
>+  // then the stroke is rendered as if a value of none were specified

In its current form I'm not sure this comment adds anything since it seems to just say what the code plainly does.

>+  if (aStyleSVG->mStrokeLinecap == NS_STYLE_STROKE_LINECAP_BUTT &&
>+      (totalLengthOfDashes <= 0 || totalLengthOfGaps <= 0)) {
>+    return (totalLengthOfGaps > 0) ? eNoStroke : eContinuousStroke;
>   }

This isn't quite correct. In the case of |totalLengthOfGaps <= 0| we want to return eContinuousStroke regardless of the value of stroke-linecap (or else we'll regress bug 609361). It's only the case where |totalLengthOfDashes <= 0| that we want to check the caps. Maybe change the code to:

  // Stroking using dashes is much slower than stroking a continuous line
  // (see bug 609361 comment 40), and much, much slower than not stroking the
  // line at all. Here we check for cases when the dash pattern causes the
  // stroke to essentially be continuous or to be nonexistent in which case
  // we can avoid expensive stroking operations (the underlying platform
  // graphics libraries don't seem to optimize for this).
  if (totalLengthOfDashes <= 0 && totalLengthOfGaps <= 0) {
    return eNoStroke;
  }
  if (totalLengthOfGaps <= 0) {
    return eContinuousStroke;
  }
  if (totalLengthOfDashes <=0) {
    // We can only return eNoStroke if the value of stroke-linecap isn't
    // adding caps to zero length dashes.
    if (aStyleSVG->mStrokeLinecap == NS_STYLE_STROKE_LINECAP_BUTT) {
      return eNoStroke;
    }
  }

I actually wonder if your test passes for all platforms though. It wouldn't surprise me if the underlying platforms have different behavior. (If so, I'd also be happy with the above code with the NS_STYLE_STROKE_LINECAP_BUTT check dropped.)
Attachment #8543708 - Attachment is obsolete: true
Attachment #8543708 - Flags: review?(jwatt)
Attachment #8546518 - Flags: review?(jwatt)
Comment on attachment 8546518 [details] [diff] [review]
addresss review comments

r+, thanks!

FWIW I personally prefer:

  if (totalLengthOfDashes <=0 ) {
    // We can only return eNoStroke if the value of stroke-linecap isn't
    // adding caps to zero length dashes.
    if (aStyleSVG->mStrokeLinecap == NS_STYLE_STROKE_LINECAP_BUTT) {
      return eNoStroke;
    }
  }

Since the compiler should do the same thing and it feels a bit clearer to me, but not a big deal by any means.
Attachment #8546518 - Flags: review?(jwatt) → review+
https://hg.mozilla.org/mozilla-central/rev/29229def6b2e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
QA Whiteboard: [good first verify]
Depends on: 1142927
Blocks: 1149516
You need to log in before you can comment on or make changes to this bug.