New issue
Advanced search Search tips

Issue 648512 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Investigate if char* would be cleaner than uintptr_t

Project Member Reported by cblume@chromium.org, Sep 20 2016

Issue description

In the code for deferred texture images, we have two main pointer requirements:
- Ability to add offsets to it
- Avoid aliasing

Those two requirements disallow void* or T*/T2*.

Right now, we use uintptr_t to store the pointer values. This solves both problems. When we need it to be a pointer again, we cast it to a void* which will not cause aliasing problems.


Another method that achieves both of these requirements is to use a char*. C and C++ have special wording for char* the prevent aliasing problems. And because a char has a size, we can use pointer math on it to add an offset.


Using char* would allow us to have fewer reinterpret_casts, which may be easier to read.

Let's make a patch and see what how both options look.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 22 2016

The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/921bc678a7b81de31b4e672326ed2f37ffb66b10

commit 921bc678a7b81de31b4e672326ed2f37ffb66b10
Author: cblume <cblume@chromium.org>
Date: Thu Sep 22 12:25:26 2016

Is char* or uintptr_t easier to read?

Using a char* instead of uintptr_t allows us to use fewer
reinterpret_casts which may make the code easier to read.

BUG= 648512 
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2356703002

Review-Url: https://codereview.chromium.org/2356703002

[modify] https://crrev.com/921bc678a7b81de31b4e672326ed2f37ffb66b10/src/image/SkImage_Gpu.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15303daf52164b1354e7cd608198decb3f31e6ca

commit 15303daf52164b1354e7cd608198decb3f31e6ca
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Thu Sep 22 19:30:00 2016

Roll src/third_party/skia/ d7a9db644..bac104605 (12 commits).

https://chromium.googlesource.com/skia.git/+log/d7a9db644496..bac104605ef3

$ git log d7a9db644..bac104605 --date=short --no-merges --format='%ad %ae %s'
2016-09-22 caryclark Reland of ix for conic fuzz (patchset #1 id:1 of https://codereview.chromium.org/2361473004/ )
2016-09-22 caryclark Revert of fix for conic fuzz (patchset #3 id:40001 of https://codereview.chromium.org/2350263003/ )
2016-09-22 brianosman Add output format properties to SkImageFilter::Context
2016-09-22 msarett Make SkColorSpaceXform::New() take bare ptrs
2016-09-22 mtklein Run commandbuffer config on CommandBuffer Perf bots.
2016-09-22 mtklein Clean up dead code.
2016-09-22 cblume Is char* or uintptr_t easier to read?
2016-09-22 benjaminwagner Add recipe support for Intel HD Graphics 530.
2016-09-22 msarett Do not quickReject until virtual drawPatch
2016-09-22 caryclark fix next kevin fuzz
2016-09-22 caryclark fix for conic fuzz
2016-09-22 caryclark speed up debug dm

BUG= 647922 , 647922 , 648512 , 647922 

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=borenet@google.com

Review-Url: https://codereview.chromium.org/2361143002
Cr-Commit-Position: refs/heads/master@{#420424}

[modify] https://crrev.com/15303daf52164b1354e7cd608198decb3f31e6ca/DEPS

Comment 3 by cblume@chromium.org, Sep 24 2016

Status: Fixed (was: Assigned)

Sign in to add a comment