fix: maxRadius to avoid overflow #183

Ovilia · 2021-12-20T07:12:37Z

This PR fixes maxRadius so that text does not overflows the shape.

Before

Take shape: 'circle' and 'diamond' for example, when text list contains too many text, it will overflows the shape.

After

Test case used:

<html>
  <head>
    <style>
      body {
        margin: 0;
      }
      #main {
        border: 1px solid red;
        width: 400px;
        height: 400px;
      }
      #circle {
        position: absolute;
        left: 0;
        top: 0;
        width: 400px;
        height: 400px;
        border-radius: 200px;
        border: 1px solid blue;
      }
      #diamond {
        position: absolute;
        left: 58px;
        top: 58px;
        width: 282.8px;
        height: 282.8px;
        transform: rotate(45deg);
        transform-origin: 50% 50%;
        border: 1px solid blue;
      }
    </style>
  </head>
  <body>
    <canvas id="main"></canvas>
    <!-- <div id="circle"></div> -->
    <div id="diamond"></div>
    <script src="./src/wordcloud2.js"></script>
    <script>
      var data = [];
      for (var i = 0; i < 400; i++) {
        data.push([
          'wordcloud'.slice(0, Math.ceil(Math.random() * 9)),
          Math.round(Math.random() * 40) + 5,
        ]);
      }

      var canvas = document.getElementById('main');
      canvas.width = 800;
      canvas.height = 800;

      WordCloud(canvas, {
        list: data,
        ellipticity: 1,
        gridSize: 4,
        shape: 'diamond',
      });
    </script>
  </body>
</html>

timdream · 2021-12-20T07:28:32Z

Thank for the contribution.

I don't think the fix is correct. Math.floor(Math.max(ngx, ngy) / 2) is the distance to the nearest edge from the center, right? First of all, center can be moved, so you would need consider that. Second of all, what if the shape is a square and we do want to fill all four corners of the canvas? The proposed maxRadius in that case will not be enough.

Feel free to propose better logic (preferably without yet another options — there are too many of these and they are conflicting sometimes, as evidenced by this case — sorry for the bad catch-all API design!

fix: maxRadius to avoid overflow

5546c35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: maxRadius to avoid overflow #183

fix: maxRadius to avoid overflow #183

Ovilia commented Dec 20, 2021

timdream commented Dec 20, 2021

fix: maxRadius to avoid overflow #183

Are you sure you want to change the base?

fix: maxRadius to avoid overflow #183

Conversation

Ovilia commented Dec 20, 2021

Before

After

Test case used:

timdream commented Dec 20, 2021